Follow-up Comment #32, bug #18776 (project freeciv):

Further eve-of-2.3.1 analysis of the issues and patches surrounding this
ticket. (Probably shortly to be split to several tickets.)


== Vision cleanup when player removed ==

file #14580: 0006-fix-bug-in-player_map_free.patch
(basically the same as file #14319 tested by akfaew in comment #14)

This is the first patched area from this bug.

_Bug:_ if I understand correctly, if a player is removed mid-game, various
consequences of its vision / tile knowledge aren't cleaned up properly,
including in data structures for remaining players. Consequences apparently
include segfaults in running games (e.g., comment #1).

Cannot have any effect if players are removed pre-game (rather than in-game),
since players haven't got any vision/knowledge at that point? Therefore, if
we're not going to fix it for 2.3.1, temporarily reverting in-game-remove
(i.e. reverting bug #18641) is a reasonable reaction, so that people don't run
into this.

_Risk/benefit:_ it touches player_map_free(). This is called by
server_remove_player(), but also from server_player_init(), which has ten call
sites, mostly to do with initial creation of players rather than removal. Call
me paranoid, but without more analysis than I have time for, I can't convince
myself that there's no risk of it breaking one of these cases, obscure (civil
war, barbarian creation) or otherwise (savegame loading), say due to tickling
a latent bug in the vision code by calling it from a new place.

*Conclusion:* Don't apply this patch for 2.3.1. Revert fix for bug #18641
(and deal with i18n consequences of doing that). Apply immediately after 2.3.1
release, to get confidence in fix.


== Player slots miscounted when loading/saving known tiles ==

file #14579:  0005-basic-fix-known-map-in-savegame2.patch
(which is basically the same as the state of play in comment #20)

_Bug:_ if the usage of player slots is sparse, then depending on luck and
alignment, you can see either:
* Segfaults or other misbehaviour, due to indexing off the end of a malloc'd
* The known-tiles status of higher-indexed players is not saved or loaded.
Such players will probably forget all the map they knew on reload. (I don't
know if that has knock-on effects elsewhere, since their player maps will
presumably have an opinion on what the terrain at those tiles is supposed to
be. Can't rule out segfaults/assertion failures.)

With the fix, I think loading from a savefile affected by the bug will
produce the following warning: "Saved game contains incomplete map data. This
can happen with old saved games, or it may indicate an invalid saved game
file. Proceed at your own risk." and carry on. That seems appropriate. Any
knock-on effects will be present in games loaded from these savegames.

Gaps in player slots can appear whenever /remove is used, whether in-game or
pre-game. (Thus, reverting bug #18641 won't hide this bug.)

_Risk/benefit:_ this change is simple and looks low-risk to me; can't see
that it can break things, especially if /remove is not used. It fixes an
observed segfault (comment #21).

*Conclusion:* Apply this "basic" patch before 2.3.1. Cross fingers.


== Divisor issue when saving/loading known tiles ==

This is the stuff in file #14574 that isn't also in file #14579 -- the *8 and
/32 confusion.

_Bug:_ I think what happens is that known information is not saved/loaded for
players with indices 40 and above; instead, zeroes are saved/loaded. (The
arithmetic starts going wrong at index 32, but indices 32..39 are saved/loaded
by luck, in the wrong bit positions; after that all the data is shifted off
the left end.)

This affects any game with >40 players (including barbarians) regardless of
whether /remove is used. It can also affect games with fewer players if
/remove is used (in-game or pre-game).

Fixing it while remaining backwards-compatible with old savefiles will be
tricky. We don't have a patch yet. There's not really any mitigation we can
apply to 2.3.1. It's not a regression from 2.3.0.

*Conclusion:* admit this as a known issue in the 2.3.1 release notes. Blocker
for 2.3.2.


Reply to this item at:


  Message sent via/by Gna!

Freeciv-dev mailing list

Reply via email to