Follow-up Comment #11, bug #19029 (project freeciv):
Updated patch attached -- targeted at S2_3 but applies to all branches. (Need
to think further about dropping unnecessary backwards compatibility for S2_4
and trunk before commit.)
I've fixed a memory leak and moved to unsigned ints in more places (the
existing bit-shifting with signed ints is not nice, although probably did the
right thing in practice).
I've added some comments acknowledging that the backwards compatibility mode
deliberately overshifts (and thus invokes undefined behaviour). I'm sure some
tool is going to castigate us for it at some future point, so a comment may
save some head-scratching.
Also, I've tweaked the semantics of the new "kvb" entries subtly, as suggested
in comment #2: a halfbyte (covering 4 player slots) is only saved/loaded if at
least one of those slots is actually in use. Previously, a game with 4 players
would save/load map-known data for 8 full maps (32 player slots) -- 8x as much
as needed. This should mitigate the increase in save time and save file size
from this workaround, and reduce load times a bit (when loading new-format
games).
(I also said in that comment that the value of "lines" is too big, wasting
memory, but I can't see what I meant now -- it looks fine to me.)
Strictly, we could avoid bloating savefiles with the "kvb" stuff entirely for
games of <=40 player slots (i.e., most of them), since the existing "k" format
is OK for such games. But the code for that would be nasty and would
complicate removing backward compatibility in future, so shrug.
A further possible refinement: for games with fog-of-war, it would be possible
to use the information in the player maps instead, per comment #5.
* Pro: for loading existing S2_3 saves, this would avoid relying on the
potentially-platform-dependent undefined behaviour.
* Pro: moving to only saving this information in one place in future savefile
formats would decrease save time/size.
* Con: this would mean that the global "known" save/load code would only be
exercised in non-fogofwar games, which are not the default. So it could easily
rot.
* Con: player maps are loaded rather later than this "known" information.
Messing around with the load sequence is fraught -- particularly on a stable
branch -- due to complex dependencies between loading steps.
Conclusion: probably not worth it, so I haven't tried to do this.
Tested with S2_3 with a game with players in slots 0 and 125 (attached):
loaded this "old" game into a "new" server, saved it, and loaded that into
both a new server and an old server, checking that both players' "known" info
is loaded OK and eyeballing the savefiles.
(On x86_64; that doesn't guarantee that backward compatibility works as
expected on Sparc, since we're talking about undefined behaviour here.)
(file #15321, file #15322)
_______________________________________________________
Additional Item Attachment:
File name: KnownSaveBitOrder-4.diff Size:7 KB
File name: idx-painted.sav.bz2 Size:14 KB
_______________________________________________________
Reply to this item at:
<http://gna.org/bugs/?19029>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
