Merge pull request #10045 from ZehMatt/fix-9994

Fix #9994: Game action tick collision during server connect and map load

distribution/changelog.txt

@@ -25,6 +25,7 @@
 - Fix: [#9955] Resizing map in while pause mode does not work and may result in freezes.
 - Fix: [#9957] When using 'no money' cheat, guests complain of running out of cash.
 - Fix: [#9970] Wait for quarter load fails.
+- Fix: [#9994] Game action tick collision during server connect and map load.
 - Fix: [#10017] Ghost elements influencing ride excitement.
 - Fix: [#10036] Do not allocate large chunks of memory for save file classification.
 - Improved: [#9466] Add the rain weather effect to the OpenGL renderer.

src/openrct2/actions/GameAction.cpp

@@ -76,6 +76,7 @@ namespace GameActions
     static GameActionFactory _actions[GAME_COMMAND_COUNT];
     static std::multiset<QueuedGameAction> _actionQueue;
     static uint32_t _nextUniqueId = 0;
+    static bool _suspended = false;
 
     GameActionFactory Register(uint32_t id, GameActionFactory factory)
     {
@@ -95,6 +96,16 @@ namespace GameActions
         return false;
     }
 
+    void SuspendQueue()
+    {
+        _suspended = true;
+    }
+
+    void ResumeQueue()
+    {
+        _suspended = false;
+    }
+
     void Enqueue(const GameAction* ga, uint32_t tick)
     {
         auto action = Clone(ga);
@@ -114,6 +125,12 @@ namespace GameActions
 
     void ProcessQueue()
     {
+        if (_suspended)
+        {
+            // Do nothing if suspended, this is usually the case between connect and map loads.
+            return;
+        }
+
         const uint32_t currentTick = gCurrentTicks;
 
         while (_actionQueue.begin() != _actionQueue.end())

src/openrct2/actions/GameAction.h

@@ -255,6 +255,14 @@ namespace GameActions
     void Register();
     bool IsValidId(uint32_t id);
 
+    // Halts the queue processing until ResumeQueue is called, any calls to ProcessQueue
+    // will have no effect during suspension. It has no effect of actions that will not
+    // cross the network.
+    void SuspendQueue();
+
+    // Resumes queue processing.
+    void ResumeQueue();
+
     void Enqueue(const GameAction* ga, uint32_t tick);
     void Enqueue(GameAction::Ptr&& ga, uint32_t tick);
     void ProcessQueue();

src/openrct2/network/Network.cpp

@@ -34,7 +34,7 @@
 // This string 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 "17"
+#define NETWORK_STREAM_VERSION "18"
 #define NETWORK_STREAM_ID OPENRCT2_VERSION "-" NETWORK_STREAM_VERSION
 
 static Peep* _pickup_peep = nullptr;
@@ -417,6 +417,7 @@ void Network::Close()
 
         client_connection_list.clear();
         GameActions::ClearQueue();
+        GameActions::ResumeQueue();
         player_list.clear();
         group_list.clear();
         _serverTickData.clear();
@@ -493,6 +494,11 @@ bool Network::BeginClient(const std::string& host, uint16_t port)
     BeginChatLog();
     BeginServerLog();
 
+    // We need to wait for the map load before we execute any actions.
+    // If the client has the title screen running then theres a potential
+    // risk of tick collision with the server map and title screen map.
+    GameActions::SuspendQueue();
+
     utf8 keyPath[MAX_PATH];
     network_get_private_key_path(keyPath, sizeof(keyPath), gConfigNetwork.player_name);
     if (!platform_file_exists(keyPath))
@@ -2615,6 +2621,8 @@ void Network::Client_Handle_MAP([[maybe_unused]] NetworkConnection& connection,
         // Start of a new map load, clear the queue now as we have to buffer them
         // until the map is fully loaded.
         GameActions::ClearQueue();
+        GameActions::SuspendQueue();
+
         _serverTickData.clear();
         _clientMapLoaded = false;
     }
@@ -2637,6 +2645,9 @@ void Network::Client_Handle_MAP([[maybe_unused]] NetworkConnection& connection,
     std::memcpy(&chunk_buffer[offset], (void*)packet.Read(chunksize), chunksize);
     if (offset + chunksize == size)
     {
+        // Allow queue processing of game actions again.
+        GameActions::ResumeQueue();
+
         context_force_close_window_by_class(WC_NETWORK_STATUS);
         bool has_to_free = false;
         uint8_t* data = &chunk_buffer[0];