From ad7ac8d59578d8074aa8d407be8c019debb2400c Mon Sep 17 00:00:00 2001 From: wraitii Date: Sun, 8 Nov 2020 08:58:19 +0000 Subject: [PATCH] Fix UpdateComponents logic for pathfinding following d592bf9cb6 Following d592bf9cb6, paths requested at turn N were set-up to be computed between the end of turn N and the start of turn N+1 (which would ultimately allow threading this computation), via calls to 'StartProcessingMoves' and 'FetchAsyncResultsAndSendMessages'. However, the call to UpdateGrid() remained at the start of turn N+1, between the 'start' and 'fetch' calls. Since all paths are currently computed on the 'start' call, this means all paths are computed on a (possibly) dirty pathfinder grid. In particular, this leads to OOS on rejoin since the rejoiner will recompute the grid before computing the outstanding paths. This would also obviously be buggy in a threaded environment, since some paths might be computed on the fresh and some on the dirty grid. Finally, MT_TurnStart was sent before the paths were computed, which might lead to further pathfinder grid changes (not a crashing problem without threading, but still conceptually odd). The 'fetch' call is thus moved before it. This thus fixes d592bf9cb6/D1918, after 92ad6a61fa already fixed a first issue. Since the grid is now only updated at the end of a turn, we need to ensure that it is correct on Turn 0, thus the pathfinder recomputes it on InitGame. Refs D14 Reported by: Itms Fixes #5851 Differential Revision: https://code.wildfiregames.com/D3064 This was SVN commit r24142. --- .../mods/public/simulation/helpers/InitGame.js | 3 +++ source/simulation2/Simulation2.cpp | 14 +++++++------- .../simulation2/components/CCmpPathfinder_Common.h | 2 -- source/simulation2/components/ICmpPathfinder.cpp | 1 + 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/binaries/data/mods/public/simulation/helpers/InitGame.js b/binaries/data/mods/public/simulation/helpers/InitGame.js index 9ad1392fb2..b2b8b945b5 100644 --- a/binaries/data/mods/public/simulation/helpers/InitGame.js +++ b/binaries/data/mods/public/simulation/helpers/InitGame.js @@ -72,6 +72,9 @@ function InitGame(settings) if (settings.WorldPopulationCap) Engine.QueryInterface(SYSTEM_ENTITY, IID_PlayerManager).SetMaxWorldPopulation(settings.WorldPopulationCap); + // Update the grid with all entities created for the map init. + Engine.QueryInterface(SYSTEM_ENTITY, IID_Pathfinder).UpdateGrid(); + // Map or player data (handicap...) dependent initialisations of components (i.e. garrisoned units). Engine.BroadcastMessage(MT_InitGame, {}); diff --git a/source/simulation2/Simulation2.cpp b/source/simulation2/Simulation2.cpp index 415236684e..a4fc939c4c 100644 --- a/source/simulation2/Simulation2.cpp +++ b/source/simulation2/Simulation2.cpp @@ -531,19 +531,16 @@ void CSimulation2Impl::UpdateComponents(CSimContext& simContext, fixed turnLengt CComponentManager& componentManager = simContext.GetComponentManager(); + CmpPtr cmpPathfinder(simContext, SYSTEM_ENTITY); + if (cmpPathfinder) + cmpPathfinder->FetchAsyncResultsAndSendMessages(); + { PROFILE2("Sim - Update Start"); CMessageTurnStart msgTurnStart; componentManager.BroadcastMessage(msgTurnStart); } - CmpPtr cmpPathfinder(simContext, SYSTEM_ENTITY); - if (cmpPathfinder) - { - cmpPathfinder->FetchAsyncResultsAndSendMessages(); - cmpPathfinder->UpdateGrid(); - } - // Push AI commands onto the queue before we use them CmpPtr cmpAIManager(simContext, SYSTEM_ENTITY); if (cmpAIManager) @@ -594,7 +591,10 @@ void CSimulation2Impl::UpdateComponents(CSimContext& simContext, fixed turnLengt // Process all remaining moves if (cmpPathfinder) + { + cmpPathfinder->UpdateGrid(); cmpPathfinder->StartProcessingMoves(false); + } } void CSimulation2Impl::Interpolate(float simFrameLength, float frameOffset, float realFrameLength) diff --git a/source/simulation2/components/CCmpPathfinder_Common.h b/source/simulation2/components/CCmpPathfinder_Common.h index 3960dbdc1e..6183fa9d42 100644 --- a/source/simulation2/components/CCmpPathfinder_Common.h +++ b/source/simulation2/components/CCmpPathfinder_Common.h @@ -89,12 +89,10 @@ public: static void ClassInit(CComponentManager& componentManager) { componentManager.SubscribeToMessageType(MT_Deserialized); - componentManager.SubscribeToMessageType(MT_Update); componentManager.SubscribeToMessageType(MT_RenderSubmit); // for debug overlays componentManager.SubscribeToMessageType(MT_TerrainChanged); componentManager.SubscribeToMessageType(MT_WaterChanged); componentManager.SubscribeToMessageType(MT_ObstructionMapShapeChanged); - componentManager.SubscribeToMessageType(MT_TurnStart); } ~CCmpPathfinder(); diff --git a/source/simulation2/components/ICmpPathfinder.cpp b/source/simulation2/components/ICmpPathfinder.cpp index 84b2a9f970..43888c38ea 100644 --- a/source/simulation2/components/ICmpPathfinder.cpp +++ b/source/simulation2/components/ICmpPathfinder.cpp @@ -25,4 +25,5 @@ BEGIN_INTERFACE_WRAPPER(Pathfinder) DEFINE_INTERFACE_METHOD_1("SetDebugOverlay", void, ICmpPathfinder, SetDebugOverlay, bool) DEFINE_INTERFACE_METHOD_1("SetHierDebugOverlay", void, ICmpPathfinder, SetHierDebugOverlay, bool) DEFINE_INTERFACE_METHOD_CONST_1("GetPassabilityClass", pass_class_t, ICmpPathfinder, GetPassabilityClass, std::string) +DEFINE_INTERFACE_METHOD_0("UpdateGrid", void, ICmpPathfinder, UpdateGrid) END_INTERFACE_WRAPPER(Pathfinder)