PBEM fails due to save game compression change from .xz to .zst

[erichelgeson]

Using develop branch at 583adba

... Configure email
vagrant up
.. provision succeeds
Start a PBEM game - welcome email sent.

Take first turn - the savegame/pbem/ dir contains a file ending in .zst

Currently the pbem.py script only handles .xz compressed files and fails to send next turn email to player

freeciv-web/pbem/pbem.py

Line 97 in c5930c2

with lzma.open(filename, mode="rt") as f:

[andreasrosdal]

Thanks for reporting, pull requests welcome.

[erichelgeson]

diff --git a/pbem/pbem.py b/pbem/pbem.py
index 12fd92b..a4ef18a 100644
--- a/pbem/pbem.py
+++ b/pbem/pbem.py
@@ -20,7 +20,10 @@

 import os, os.path
 import time
-import lzma
+#import lzma
+import zstandard as zstd
+import tempfile
 import mysql.connector
 from mailsender import MailSender
 from mailstatus import *
@@ -94,14 +97,20 @@ def handle_savegame(root, file):
   print("Handling savegame: " + filename);
   txt = None;
   try:
-    with lzma.open(filename,  mode="rt") as f:
-      txt = f.read().split("\n");
-      status.savegames_read += 1;
+    with open(filename, 'rb') as compressed:
+        decomp = zstd.ZstdDecompressor()
+        output_path = tempfile.NamedTemporaryFile().name
+        with open(output_path, 'wb') as destination:
+            decomp.copy_stream(compressed, destination)
+    with open(output_path, "rt") as f:
+        txt = f.read().split("\n");
+        status.savegames_read += 1;
   except Exception as inst:
     print("Error loading savegame: " + str(inst));
     return;

-  new_filename = "pbem_processed_" + str(random.randint(0,10000000000)) + ".xz";
+  new_filename = "pbem_processed_" + str(random.randint(0,10000000000)) + ".zst";
   f.close();
   if not testmode: shutil.move(filename, os.path.join(root,new_filename))
   print("New filename will be: " + new_filename);
@@ -220,7 +229,7 @@ def save_game_result(winner, playerOne, playerTwo):
 def process_savegames():
   for root, subFolders, files in os.walk(savedir):
     for file in files:
-      if (file.endswith(".xz") and file.startswith("pbem") and not file.startswith("pbem_processed")):
+      if (file.endswith(".zst") and file.startswith("pbem") and not file.startswith("pbem_processed")):
         handle_savegame(root, file);

Here is a very trivial patch in that it does not handle any migrations, and creates a temp file - zstandard does not have the same read() like lzma does. It appears the entire file is loaded into memory anyways and I'd think they are small so this may be OK. Any guidance would be appreciated - more of a Java dev then python.

[erichelgeson]

Unfortunately this only partially works - while the game saves after the first turn to a zst file - when a player clicks a link to take the next turn it is looking for a xz file... I'm not sure where that is handled.

[andreasrosdal]

https://github.com/freeciv/freeciv-web/blob/develop/freeciv-web/src/main/webapp/javascript/pbem.js#L514

https://github.com/freeciv/freeciv-web/blob/develop/pbem/pbem.py#L125

Please submit a propser GitHub pull request.

[andreasrosdal]

Or revert to using .xz compression format in Freeciv-web. The differences between the various compession formats is usually minimal these days.

[erichelgeson]

I'm trying to trace how this all works and as someone unfamiliar with the codebase, it's quite complex. So the pbem.js script is sending a websocket message to a python proxy, which is sending that save game name to an actual freeciv instance. It seems like I could recompile with compresstype set to xz possibly, though I'm not familiar with this build/make/meson setup to know where the correct location is, or even if this is a compile time setting.

[andreasrosdal]

@cazfi

[cazfi]

I've meant to work on implementing the zstd support, but thought that all game types currently hardcode .xz to use in their .serv files (compresstype server setting). That might not be the case for pbem games?
Another quick solution is to revert installing libzstd-dev to freeciv-web, so that the server build will not find it.

[cazfi]

Hmm... publite2/pubscript_pbem.serv has "set compresstype xz" - isn't that one currently used for pbem games?

[erichelgeson]

Ahh, so that was the issue - I was switching the rule set as I thought (maybe incorrectly) that for web games I should be using Webperimental - if i try to start a game with that ruleset the compression is .zst and attempting to change that setting it says "you are not allowed to change the setting compresstype" - which makes sense for users not to do that.

I guess the fix is to ensure any rulesets used by the web save in the correct compresstype. Though, looking at the rulesets/.serv files - I'm not sure the correct way to go.

Edit: Would adding set compresstype xz to freeciv/freeciv/data/webperimental.serv be the right solution/work around? does freeciv need a recompile to pick up these files?

[cazfi]

This seems to be a bigger problem than just the compresstype then. Freeciv-web (publite) seems to be designed with the assumption that settings set in the .serv file it initially feeds to the server remain in effect. But that's not the case if user changes ruleset, causing settings to reset to server's defaults.
We can't really patch individual ruleset .serv files, as 1) those publite2/*.serv files are different between pbem/singleplayer/multiplayer -> for some settings, there's not a single value that would be correct in all cases, 2) are probably never read anyway (/rulesetdir just changes the ruleset, it does not run any .serv that would do that) To resolve (2) we could change settings list in game.ruleset, but I don't think that's a step toward proper fix.

Maybe we should have some feature in the server for it to reread the initial .serv file when ever the settings get resetted.

[cazfi]

Some of those "game type" .serv files under publite also want to set specific rulesetdir, so in those cases changing the ruleset should not be allowed at all.
Likely we should now just disallow changing ruleset, and enable it only when we have a bunch of new server features to support all this.

[cazfi]

we should now just disallow changing ruleset

#485