Fixed few multiplayer desync issues. (#5578)

This addresses some of the desync causes:

* `vehicle_create_car` was using `scenario_rand` when it shouldn't have
* ghost elements affected grass growth
* ghost elements affecting peep logic[1]

It also adds some desync debug facilities, enabled at compile time.

It also reverts part of change introduced in
https://github.com/OpenRCT2/OpenRCT2/pull/5185,
namely reorder of desync check vs call to `ProcessGameCommandQueue();`

[1] It is not ideal to have this check in multiple locations, it is prone
to human error. We already have `map_remove_provisional_elements`,
but it is possible it does not work as well as it should. This needs
further investigation.

contributors.md

@@ -78,6 +78,7 @@ Includes all git commit authors. Aliases are GitHub user names.
 * Marco Benzi Tobar (Lisergishnu)
 * Richard Jenkins (rwjuk)
 * (ceeac)
+* Matthias Moninger (Zeh Matt)
 
 ## Toolchain
 * (Balletie) - macOS

src/openrct2/game.c

@@ -305,6 +305,10 @@ void game_update()
         // Update the animation list. Note this does not
         // increment the map animation.
         map_animation_invalidate_all();
+
+        // Special case because we set numUpdates to 0, otherwise in game_logic_update.
+        game_handle_input();
+        network_update();
     }
 
     // Update the game one or more times
@@ -337,6 +341,28 @@ void game_update()
     window_dispatch_update_all();
 
     gGameCommandNestLevel = 0;
+}
+
+void game_logic_update()
+{
+
+    ///////////////////////////
+    gInUpdateCode = true;
+    ///////////////////////////
+
+    network_update();
+
+    if (network_get_mode() == NETWORK_MODE_CLIENT && network_get_status() == NETWORK_STATUS_CONNECTED && network_get_authstatus() == NETWORK_AUTH_OK) {
+        // Can't be in sync with server, round trips won't work if we are at same level.
+        if (gCurrentTicks >= network_get_server_tick()) {
+            // Don't run past the server
+            return;
+        }
+    }
+
+    gScreenAge++;
+    if (gScreenAge == 0)
+        gScreenAge--;
 
     if (!gOpenRCT2Headless)
     {
@@ -366,25 +392,6 @@ void game_update()
         gUnk141F568 = gUnk13CA740;
         game_handle_input();
     }
-}
-
-void game_logic_update()
-{
-    ///////////////////////////
-    gInUpdateCode = true;
-    ///////////////////////////
-    network_update();
-    if (network_get_mode() == NETWORK_MODE_CLIENT && network_get_status() == NETWORK_STATUS_CONNECTED && network_get_authstatus() == NETWORK_AUTH_OK) {
-        if (gCurrentTicks >= network_get_server_tick()) {
-            // Don't run past the server
-            return;
-        }
-    }
-    gCurrentTicks++;
-    gScenarioTicks++;
-    gScreenAge++;
-    if (gScreenAge == 0)
-        gScreenAge--;
 
     sub_68B089();
     scenario_update();
@@ -403,6 +410,7 @@ void game_logic_update()
     ride_ratings_update_all();
     ride_measurements_update();
     news_item_update_current();
+
     ///////////////////////////
     gInUpdateCode = false;
     ///////////////////////////
@@ -413,8 +421,6 @@ void game_logic_update()
     climate_update_sound();
     editor_open_windows_for_current_step();
 
-    gSavedAge++;
-
     // Update windows
     //window_dispatch_update_all();
 
@@ -431,8 +437,13 @@ void game_logic_update()
     }
 
     // Start autosave timer after update
-    if (gLastAutoSaveUpdate == AUTOSAVE_PAUSE)
+    if (gLastAutoSaveUpdate == AUTOSAVE_PAUSE) {
         gLastAutoSaveUpdate = platform_get_ticks();
+    }
+
+    gCurrentTicks++;
+    gScenarioTicks++;
+    gSavedAge++;
 }
 
 /**
@@ -561,7 +572,8 @@ sint32 game_do_command_p(sint32 command, sint32 *eax, sint32 *ebx, sint32 *ecx,
 
             // Do the callback (required for multiplayer to work correctly), but only for top level commands
             if (gGameCommandNestLevel == 1) {
-                if (game_command_callback && !(flags & GAME_COMMAND_FLAG_GHOST)) {
+                if (game_command_callback && !(flags & GAME_COMMAND_FLAG_GHOST)) 
+                {
                     game_command_callback(*eax, *ebx, *ecx, *edx, *esi, *edi, *ebp);
                     game_command_callback = 0;
                 }
@@ -1399,6 +1411,8 @@ void game_load_or_quit_no_save_prompt()
  */
 void game_init_all(sint32 mapSize)
 {
+    gInUpdateCode = true;
+
     map_init(mapSize);
     park_init();
     finance_init();
@@ -1412,6 +1426,8 @@ void game_init_all(sint32 mapSize)
     news_item_init_queue();
     user_string_clear_all();
 
+    gInUpdateCode = false;
+
     window_new_ride_init_vars();
     window_guest_list_init_vars_a();
     window_guest_list_init_vars_b();

src/openrct2/network/network.cpp

@@ -520,8 +520,11 @@ void Network::UpdateClient()
         }
 
         // Check synchronisation
+        ProcessGameCommandQueue();
+
         if (!_desynchronised && !CheckSRAND(gCurrentTicks, gScenarioSrand0)) {
             _desynchronised = true;
+
             char str_desync[256];
             format_string(str_desync, 256, STR_MULTIPLAYER_DESYNC, NULL);
             window_network_status_open(str_desync, NULL);
@@ -530,7 +533,6 @@ void Network::UpdateClient()
             }
         }
 
-        ProcessGameCommandQueue();
         break;
     }
     }
@@ -614,12 +616,17 @@ bool Network::CheckSRAND(uint32 tick, uint32 srand0)
         return true;
     }
 
-    if (tick == server_srand0_tick) {
+    if (tick == server_srand0_tick) 
+    {
         server_srand0_tick = 0;
         // Check that the server and client sprite hashes match
-        const bool sprites_mismatch = server_sprite_hash[0] != '\0' && strcmp(sprite_checksum(), server_sprite_hash);
+        const char *client_sprite_hash = sprite_checksum();
+        const bool sprites_mismatch = server_sprite_hash[0] != '\0' && strcmp(client_sprite_hash, server_sprite_hash);
         // Check PRNG values and sprite hashes, if exist
         if ((srand0 != server_srand0) || sprites_mismatch) {
+#ifdef DEBUG_DESYNC
+            dbg_report_desync(tick, srand0, server_srand0, client_sprite_hash, server_sprite_hash);
+#endif
             return false;
         }
     }
@@ -1315,7 +1322,13 @@ void Network::ProcessPacket(NetworkConnection& connection, NetworkPacket& packet
 
 void Network::ProcessGameCommandQueue()
 {
-    while (game_command_queue.begin() != game_command_queue.end() && game_command_queue.begin()->tick == gCurrentTicks) {
+    while (game_command_queue.begin() != game_command_queue.end()) {
+        // If our tick is higher than the command tick we are in trouble.
+        assert(game_command_queue.begin()->tick >= gCurrentTicks);
+
+        if (game_command_queue.begin()->tick != gCurrentTicks)
+            return;
+
         // run all the game commands at the current tick
         const GameCommand& gc = (*game_command_queue.begin());
         if (GetPlayerID() == gc.playerid) {

src/openrct2/network/network.h

@@ -56,7 +56,7 @@ extern "C" {
 // This define specifies which version of network stream current build uses.
 // It is used for making sure only compatible builds get connected, even within
 // single OpenRCT2 version.
-#define NETWORK_STREAM_VERSION "10"
+#define NETWORK_STREAM_VERSION "11"
 #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION
 
 #ifdef __cplusplus

src/openrct2/rct2.c

@@ -386,7 +386,7 @@ void rct2_update()
 
     // TODO: screenshot countdown process
 
-    network_update();
+    // network_update() is called in game_update
 
     // check_cmdline_arg();
     // Screens

src/openrct2/scenario/scenario.c

@@ -566,20 +566,84 @@ static sint32 scenario_create_ducks()
  *
  * @return eax
  */
+#ifndef DEBUG_DESYNC 
 uint32 scenario_rand()
+#else
+static FILE *fp = NULL;
+static const char *realm = "LC";
+
+uint32 dbg_scenario_rand(const char *file, const char *function, const uint32 line, const void *data)
+#endif
 {
+    uint32 originalSrand0 = gScenarioSrand0;
+    gScenarioSrand0 += ror32(gScenarioSrand1 ^ 0x1234567F, 7);
+    gScenarioSrand1 = ror32(originalSrand0, 3);
+
 #ifdef DEBUG_DESYNC
+    if (fp == NULL)
+    {
+        if (network_get_mode() == NETWORK_MODE_SERVER)
+        {
+            fp = fopen("server_rand.txt", "wt");
+            realm = "SV";
+        }
+        else if (network_get_mode() == NETWORK_MODE_CLIENT)
+        {
+            fp = fopen("client_rand.txt", "wt");
+            realm = "CL";
+        }
+        else
+        {
+            if (fp)
+                fclose(fp);
+            fp = NULL;
+            realm = "LC";
+        }
+    }
+    if (fp)
+    {
+        fprintf(fp, "Tick: %d, Rand: %08X - REF: %s:%u %s (%p)\n", gCurrentTicks, gScenarioSrand1, file, line, function, data);
+    }
     if (!gInUpdateCode) {
         log_warning("scenario_rand called from outside game update");
         assert(false);
     }
 #endif
 
-    uint32 originalSrand0 = gScenarioSrand0;
-    gScenarioSrand0 += ror32(gScenarioSrand1 ^ 0x1234567F, 7);
-    return gScenarioSrand1 = ror32(originalSrand0, 3);
+    return gScenarioSrand1;
 }
 
+#ifdef DEBUG_DESYNC 
+void dbg_report_desync(uint32 tick, uint32 srand0, uint32 server_srand0, const char *clientHash, const char *serverHash)
+{
+    if (fp == NULL)
+    {
+        if (network_get_mode() == NETWORK_MODE_SERVER)
+        {
+            fp = fopen("server_rand.txt", "wt");
+            realm = "SV";
+        }
+        else if (network_get_mode() == NETWORK_MODE_CLIENT)
+        {
+            fp = fopen("client_rand.txt", "wt");
+            realm = "CL";
+        }
+    }
+    if (fp)
+    {
+        const bool sprites_mismatch = serverHash[0] != '\0' && strcmp(clientHash, serverHash);
+
+        fprintf(fp, "[%s] !! DESYNC !! Tick: %d, Client Hash: %s, Server Hash: %s, Client Rand: %08X, Server Rand: %08X - %s\n", realm, 
+                tick, 
+                clientHash, 
+                ( (serverHash[0] != '\0') ? serverHash : "<NONE:0>" ),
+                srand0,
+                server_srand0,
+                (sprites_mismatch ? "Sprite hash mismatch" : "scenario rand mismatch"));
+    }
+}
+#endif
+
 uint32 scenario_rand_max(uint32 max)
 {
     if (max < 2) return 0;

src/openrct2/scenario/scenario.h

@@ -393,8 +393,18 @@ sint32 scenario_load(const char *path);
 sint32 scenario_load_and_play_from_path(const char *path);
 void scenario_begin();
 void scenario_update();
+
+#ifdef DEBUG_DESYNC 
+uint32 dbg_scenario_rand(const char *file, const char *function, const uint32 line, const void *data);
+#define scenario_rand() dbg_scenario_rand(__FILE__, __FUNCTION__, __LINE__, NULL)
+#define scenario_rand_data(data) dbg_scenario_rand(__FILE__, __FUNCTION__, __LINE__, data)
+void dbg_report_desync(uint32 tick, uint32 srand0, uint32 server_srand0, const char *clientHash, const char *serverHash);
+#else 
 uint32 scenario_rand();
+#endif
+
 uint32 scenario_rand_max(uint32 max);
+
 sint32 scenario_prepare_for_save();
 sint32 scenario_save(const utf8 * path, sint32 flags);
 void scenario_remove_trackless_rides(rct_s6_data *s6);

src/openrct2/world/map.c

@@ -3719,6 +3719,9 @@ static void map_update_grass_length(sint32 x, sint32 y, rct_map_element *mapElem
             mapElementAbove++;
             if (map_element_get_type(mapElementAbove) == MAP_ELEMENT_TYPE_WALL)
                 continue;
+            // Grass should not be affected by ghost elements.
+            if (map_element_is_ghost(mapElementAbove))
+                continue;
             if (z0 >= mapElementAbove->clearance_height)
                 continue;
             if (z1 < mapElementAbove->base_height)