address review comments

Author: bruno-dasilva

rts/Lua/LuaConstEngine.cpp

@@ -28,7 +28,6 @@
  * @field noHandicapForReclaim boolean Whether handicap is applied to income from reclaim
  * @field groupAddDoesntSelect boolean Whether 'group add' also selects the group (does both if false)
  * @field deadTeamsKeepUnitLimit boolean Whether engine redistributes dead team unitlimit to allies (false) or keeps it as-is (true)
- * @field hasChecksums boolean Whether the engine was built with SYNCCHECK (i.e. Spring.GetPrevFrameChecksum() returns a value)
  */
 
 /***
@@ -47,6 +46,7 @@
  * @field gameSpeed number Number of simulation gameframes per second
  * @field textColorCodes TextColorCode Table containing keys that represent the color code operations during font rendering
  * @field isHeadless boolean? Whether this is a headless engine build. Not available in synced
+ * @field hasSyncChecksums boolean? Whether the engine was built with sync-check support (i.e. Spring.GetPrevFrameSyncChecksum() returns a meaningful value). Not available in synced
  */
 
 bool LuaConstEngine::PushEntries(lua_State* L)
@@ -60,8 +60,14 @@ bool LuaConstEngine::PushEntries(lua_State* L)
 	LuaPushNamedString(L, "buildFlags"     , SpringVersion::GetAdditional());
 	LuaPushNamedNumber(L, "wordSize", (!CLuaHandle::GetHandleSynced(L))? Platform::NativeWordSize() * 8: 0);
 
-	if (!CLuaHandle::GetHandleSynced(L))
+	if (!CLuaHandle::GetHandleSynced(L)) {
 		LuaPushNamedBool(L, "isHeadless", SpringVersion::IsHeadless());
+#ifdef SYNCCHECK
+		LuaPushNamedBool(L, "hasSyncChecksums", true);
+#else
+		LuaPushNamedBool(L, "hasSyncChecksums", false);
+#endif
+	}
 
 	LuaPushNamedNumber(L, "gameSpeed", GAME_SPEED);
 	LuaPushNamedNumber(L, "maxCustomPaletteID", MAX_CUSTOM_COLORS - 1);
@@ -74,7 +80,7 @@ bool LuaConstEngine::PushEntries(lua_State* L)
 	 *
 	 * will be compatible even on engines that don't yet know about the entry at all. */
 	lua_pushliteral(L, "FeatureSupport");
-	lua_createtable(L, 0, 12);
+	lua_createtable(L, 0, 11);
 		LuaPushNamedBool(L, "NegativeGetUnitCurrentCommand", true);
 		LuaPushNamedBool(L, "hasExitOnlyYardmaps", true);
 		LuaPushNamedNumber(L, "rmlUiApiVersion", 1);
@@ -88,11 +94,6 @@ bool LuaConstEngine::PushEntries(lua_State* L)
 		LuaPushNamedBool(L, "noHandicapForReclaim", true);
 		LuaPushNamedBool(L, "groupAddDoesntSelect", true);
 		LuaPushNamedBool(L, "deadTeamsKeepUnitLimit", false);
-#ifdef SYNCCHECK
-		LuaPushNamedBool(L, "hasChecksums", true);
-#else
-		LuaPushNamedBool(L, "hasChecksums", false);
-#endif
 	lua_rawset(L, -3);
 
 	lua_pushliteral(L, "textColorCodes");

rts/Lua/LuaSyncedRead.cpp

@@ -79,7 +79,6 @@
 #include "System/FileSystem/FileHandler.h"
 #include "System/FileSystem/FileSystem.h"
 #include "System/StringUtil.h"
-#include "System/Sync/SyncChecker.h"
 
 #include <cctype>
 #include <functional>
@@ -122,7 +121,6 @@ bool LuaSyncedRead::PushEntries(lua_State* L)
 
 	REGISTER_LUA_CFUNC(GetGameFrame);
 	REGISTER_LUA_CFUNC(GetGameSeconds);
-	REGISTER_LUA_CFUNC(GetPrevFrameChecksum);
 
 	REGISTER_LUA_CFUNC(GetGameRulesParam);
 	REGISTER_LUA_CFUNC(GetGameRulesParams);
@@ -933,39 +931,6 @@ int LuaSyncedRead::GetGameSeconds(lua_State* L)
 }
 
 
-/***
- *
- * Returns the engine's final sync checksum for the previous simulation
- * frame as an 8-character lowercase hex string (e.g. `"fade1eaf"`).
- * The value is stable for the entire duration of the current frame
- * (independent of gadget/callin ordering) and is identical across all
- * in-sync clients, so it can be folded into a deterministic per-run
- * artifact for sync testing (e.g. fightertest).
- *
- * Returned as a string rather than a number because Spring's Lua is
- * built with `LUA_NUMBER = float`, which cannot represent a 32-bit
- * checksum losslessly.
- *
- * Returns `nil` on builds without `SYNCCHECK`. Check
- * `Engine.FeatureSupport.hasChecksums` to detect availability.
- *
- * @function Spring.GetPrevFrameChecksum
- *
- * @return string|nil checksum
- */
-int LuaSyncedRead::GetPrevFrameChecksum(lua_State* L)
-{
-#ifdef SYNCCHECK
-	char buf[16];
-	snprintf(buf, sizeof(buf), "%08x", CSyncChecker::GetPrevChecksum());
-	lua_pushstring(L, buf);
-#else
-	lua_pushnil(L);
-#endif
-	return 1;
-}
-
-
 /******************************************************************************
  * Environment
  * @section environment

rts/Lua/LuaSyncedRead.h

@@ -45,7 +45,6 @@ class LuaSyncedRead {
 
 		static int GetGameFrame(lua_State* L);
 		static int GetGameSeconds(lua_State* L);
-		static int GetPrevFrameChecksum(lua_State* L);
 
 		static int GetGameRulesParam(lua_State* L);
 		static int GetGameRulesParams(lua_State* L);

rts/Lua/LuaUnsyncedRead.cpp

@@ -75,6 +75,7 @@
 #include "System/Sound/ISound.h"
 #include "System/Sound/ISoundChannels.h"
 #include "System/StringUtil.h"
+#include "System/Sync/SyncChecker.h"
 #include "System/Misc/SpringTime.h"
 #include "System/ScopedResource.h"
 #include "System/Math/NURBS.h"
@@ -120,6 +121,7 @@ bool LuaUnsyncedRead::PushEntries(lua_State* L)
 	REGISTER_LUA_CFUNC(GetGameSecondsInterpolated);
 	REGISTER_LUA_CFUNC(GetLastUpdateSeconds);
 	REGISTER_LUA_CFUNC(GetVideoCapturingMode);
+	REGISTER_LUA_CFUNC(GetPrevFrameSyncChecksum);
 
 	REGISTER_LUA_CFUNC(GetNumDisplays);
 	REGISTER_LUA_CFUNC(GetViewGeometry);
@@ -1236,6 +1238,35 @@ int LuaUnsyncedRead::GetVideoCapturingMode(lua_State* L)
 }
 
 
+/***
+ *
+ * Returns the engine's sync checksum for the previous simframe,
+ * useful for testing. The returned string is NOT convertible to
+ * a number within Lua.
+ *
+ * Returns a dummy value if `Engine.hasSyncChecksums` is false,
+ * or if no frames were processed yet.
+ *
+ * @function Spring.GetPrevFrameSyncChecksum
+ *
+ * @return string checksum
+ */
+int LuaUnsyncedRead::GetPrevFrameSyncChecksum(lua_State* L)
+{
+#ifdef SYNCCHECK
+	unsigned checksum = CSyncChecker::GetPrevChecksum();
+#else
+	unsigned checksum = 0;
+#endif
+
+	char buf[9];
+	snprintf(buf, sizeof(buf), "%08x", checksum);
+	lua_pushstring(L, buf);
+
+	return 1;
+}
+
+
 /******************************************************************************
  * Unit attributes
  * @section unitattributes

rts/Lua/LuaUnsyncedRead.h

@@ -32,6 +32,7 @@ class LuaUnsyncedRead {
 		static int GetGameSecondsInterpolated(lua_State* L);
 		static int GetLastUpdateSeconds(lua_State* L);
 		static int GetVideoCapturingMode(lua_State* L);
+		static int GetPrevFrameSyncChecksum(lua_State* L);
 
 		static int GetNumDisplays(lua_State* L);
 		static int GetViewGeometry(lua_State* L);

rts/Net/NetCommands.cpp

@@ -623,9 +623,9 @@ void CGame::ClientReadNet()
 				ASSERT_SYNCED(CSyncChecker::GetChecksum());
 				clientNet->Send(CBaseNetProtocol::Get().SendSyncResponse(gu->myPlayerNum, gs->frameNum, CSyncChecker::GetChecksum()));
 
-				// Cache the just-sent, frame-closed checksum so Lua gadgets
-				// running in the NEXT sim frame can read a stable value via
-				// Spring.GetPrevFrameChecksum(). Must happen before the
+				// Cache the just-closed frame's checksum so Lua can read a
+				// stable value during the next sim frame via
+				// Spring.GetPrevFrameSyncChecksum(). Must run before the
 				// 4096-frame reset below so we capture the pre-reset value.
 				CSyncChecker::SetPrevChecksum(CSyncChecker::GetChecksum());