Ticket #44778

Resolve script dangers around unit_change_owner()

Date d'ouverture: 2022-06-08 15:48 Dernière mise à jour: 2022-06-16 13:29

Rapporteur:
Propriétaire:
Type:
État:
Atteints
Composant:
Priorité:
5 - moyen
Sévérité:
5 - moyen
Résolution:
Fixed
Fichier:
6

Détails

A split of #44738.

It should be handled that this function sometimes does not give out a unit due to the callback in wipe_init(). In do_capture_units() stack iteration must be safe. Also, in diplomat_bribe() we should avoid getting into negative gold (what's the alternative?..)

Ticket History (3/15 Histories)

2022-06-08 15:48 Updated by: ihnatus
  • New Ticket "Resolve script dangers around unit_change_owner()" created
2022-06-10 06:39 Updated by: ihnatus
2022-06-10 06:39 Updated by: ihnatus
  • File 3.0-fix-unit_change_owner.patch (File ID: 9440) is deleted
2022-06-10 06:46 Updated by: ihnatus
Commentaire

A 3.0 patch is ready, it works for simple cases, though really nasty callbacks I could write still fail due to code out of scope (specifically, clearing of vision in server_remove_unit() before calling a callback that makes it not recursive); some changes done in previous patches in HRM849859 and #44312 are needed for it but the patches theselves should be modified to apply over or beneath this one.

2022-06-13 06:29 Updated by: ihnatus
Commentaire

Prepared a 3.1 patch. It probably should apply to 3.2 but maybe we should rewrite 3.2 unit_change_owner() completely by HRM874201.

2022-06-13 14:55 Updated by: cazfi
Commentaire

Reply To ihnatus

Prepared a 3.1 patch. It probably should apply to 3.2

Yes, it applies to master too.

---

- non_allied_not_listed_at() header begins with:
/*******************************************************************//*****
There should be no "*//*" part in S3_0, and in later branches it should be almost in the end of the line - unify with the other headers in the same source file (they are not unified over different source files)

- First line of the commit message should be one line summary (that shows up as heading in various places) followed by one empty line, and the full description after that. I've been adjusting the commit messages anyway, but less I need to do that, the better.

2022-06-14 03:22 Updated by: ihnatus
Commentaire

Fixed. Also, added a notice on stdout (not shown to the players) if the gold is underflown.

2022-06-14 15:26 Updated by: cazfi
  • Propriétaire Update from (Aucun) to cazfi
  • Résolution Update from Aucun to Accepted
2022-06-16 13:26 Updated by: cazfi
Commentaire

Reply To ihnatus

Fixed. Also, added a notice on stdout (not shown to the players) if the gold is underflown.

Clarification: This is freeciv logging (like it should), and not necessarily stdout.

2022-06-16 13:29 Updated by: cazfi
  • État Update from Ouvert to Atteints
  • Résolution Update from Accepted to Fixed

Attachment File List

Modifier

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Connexion