Let a cached route field notice the ground moved, wherever it moved

.github/workflows/build.yml

@@ -154,6 +154,11 @@ jobs:
           scons -j$(nproc) release=1 server=0 building-expel-test
           timeout 300s ./build/src/BuildingExpelHarness
 
+      - name: Build and run the building gradient invalidation regression
+        run: |
+          scons -j$(nproc) release=1 server=0 building-gradient-invalidation-test
+          ./build/src/BuildingGradientInvalidationHarness
+
       - name: Build and run the hiring bucket regression
         run: |
           scons -j$(nproc) release=1 server=0 hiring-bucket-test

src/EngineTiming.h

@@ -96,3 +96,19 @@ static constexpr int MINIMAP_REFRESH_TICKS = 25;
 //! TypeSteps.cpp.
 static constexpr int CLEARING_FLAG_REFRESH_TICKS = 125;
 
+//! A building's route field, once invalidated by a map change, is rebuilt on
+//! its next use at most this often (~4 s). The map's topology generation
+//! invalidates every field of every team on any structural change, so this
+//! interval, not the invalidation, is what bounds the rebuild load.
+//! Measured on gd-bigarena-long (Oazis, 11 teams) over 8000 ticks: at 25 the
+//! topology generation costs +42.6% simulation time over a build without it;
+//! at 100 that falls to +10.3% while the symptom it exists to fix stays fully
+//! suppressed (0 stuck-unit forced rebuilds, against 195 without it). The fix
+//! erodes past ~200 (57 stuck at 200, 76 at 400, converging on 195), so 100
+//! sits inside the safe range rather than at its edge. Same shape on
+//! gd-large-4ai and at 4000 ticks. Shared with
+//! test/BuildingGradientInvalidationHarness.cpp, which has to let this
+//! interval elapse before it can judge a field. See
+//! MapPathfindBuilding.cpp.
+static constexpr unsigned int GRADIENT_DIRTY_REBUILD_TICKS = 100;
+

src/FileFormatVersions.h

@@ -92,6 +92,10 @@ static constexpr int FILE_FORMAT_VERSION_CONTINUATION_STATE = 91;
 //! A building's round-trip fields and gradient use stamps join the cached routing fields.
 static constexpr int FILE_FORMAT_VERSION_ROUND_TRIP_FIELDS = 95;
 
+//! The map's topology generation and each cached field's generation stamp
+//! (MapIO.cpp:350, 410, 451, 514).
+static constexpr int FILE_FORMAT_VERSION_TOPOLOGY_GENERATION = 97;
+
 // === Save-file section signatures (4-byte ASCII tags) ===
 // Embedded as four chars at the start of each save section so a corrupted
 // stream fails fast. NEVER change these values — old saves on disk depend

src/Game.h

@@ -111,12 +111,6 @@ static constexpr int SLOT_INDEX_NONE = -1;
 //! See Game_editor.cpp:113, 139.
 static constexpr float TEAM_COLOR_HUE_DEGREES = 360.0f;
 
-//! Padding (in tiles) added on each side of the rectangle passed to
-//! Map::dirtyBuildingGradients when a building/flag changes. The width/height
-//! of the dirty rect therefore grows by 2 * GRADIENT_DIRTY_BORDER_TILES.
-//! See Game_orders.cpp:193, 279, 360, 496.
-static constexpr int GRADIENT_DIRTY_BORDER_TILES = 16;
-
 class Game
 {
 	bool hasSavedRandomState = false;

src/Game_io.cpp

@@ -312,6 +312,16 @@ bool Game::load(GAGCore::InputStream *stream)
 	return true;
 }
 
+// Known gap, deliberately out of scope here: this writes into the building tile
+// grid directly rather than through Map::setBuilding, so it does not bump
+// Map::topologyGeneration, and moving the bump here would not help - integrity()
+// runs before Map::loadRuntimeState, which then restores the saved generation and
+// the per-field stamps over anything set while healing. A save whose grid was
+// already inconsistent therefore gets healed and its restored fields treated as
+// current against a map the heal changed. It predates the generation (fields were
+// restored after the heal without being dirtied before it too) and only fires for
+// saves that were already inconsistent. A fix has to record that the heal touched
+// a cell and bump after loadRuntimeState.
 bool Game::checkBuildingsDoNotOverlapAndHealMissing() {
 	std::vector<Uint16> buildings(map.getW()*map.getH(), NOGBID);
 	for (int ti=0; ti<mapHeader.getNumberOfTeams(); ti++)

src/Game_orders.cpp

@@ -169,14 +169,12 @@ void Game::executeCreate(const OrderCreate& oc, int localPlayer)
 		buildProject.unitWorking = oc.unitWorking;
 		buildProject.unitWorkingFuture = oc.unitWorkingFuture;
 		buildProjects.push_back(buildProject);
-		Uint32 teamMask=Team::teamNumberToMask(oc.teamNumber);
 		for (int y=posY; y<posY+h; y++)
 			for (int x=posX; x<posX+w; x++)
 			{
-				size_t index=(x&map.wMask)+(((y&map.hMask)<<map.wDec));
-				map.tiles[index].forbidden|=teamMask;
+				map.addForbidden(x, y, oc.teamNumber);
 				if (oc.teamNumber == players[localPlayer]->teamNumber)
-					map.displayedForbiddenView.set(index, true);
+					map.displayedForbiddenView.set(map.coordToIndex(x, y), true);
 			}
 		map.updateForbiddenGradient(oc.teamNumber);
 	}
@@ -217,10 +215,7 @@ void Game::executeModifyFlag(const OrderModifyFlag& omf, int localPlayer)
 		if (b->type->zonableForbidden)
 		{
 			if (newRange<oldRange)
-			{
 				b->owner->dirtyGlobalGradient();
-				map.dirtyBuildingGradients(b->posX-oldRange-GRADIENT_DIRTY_BORDER_TILES, b->posY-oldRange-GRADIENT_DIRTY_BORDER_TILES, 2*GRADIENT_DIRTY_BORDER_TILES+oldRange*2, 2*GRADIENT_DIRTY_BORDER_TILES+oldRange*2, b->owner->teamNumber);
-			}
 		}
 		else
 		{
@@ -266,12 +261,6 @@ void Game::executeMoveFlag(const OrderMoveFlag& omf, int localPlayer)
 	Building *b=lookupBuilding(omf.gid);
 	if ((b) && (b->buildingState==Building::ALIVE) && (b->type->isVirtual))
 	{
-		if (drop && b->type->zonableForbidden)
-		{
-			int range=b->unitStayRange;
-			map.dirtyBuildingGradients(b->posX-range-GRADIENT_DIRTY_BORDER_TILES, b->posY-range-GRADIENT_DIRTY_BORDER_TILES, 2*GRADIENT_DIRTY_BORDER_TILES+range*2, 2*GRADIENT_DIRTY_BORDER_TILES+range*2, b->owner->teamNumber);
-		}
-
 		b->posX=omf.x;
 		b->posY=omf.y;
 
@@ -291,46 +280,41 @@ void Game::executeAlterForbidden(const OrderAlterForbidden& oaa, int localPlayer
 {
 	if (oaa.type == BrushTool::MODE_ADD)
 	{
-		Uint32 teamMask = Team::teamNumberToMask(oaa.teamNumber);
 		size_t orderMaskIndex = 0;
 		for (int y=oaa.centerY+oaa.minY; y<oaa.centerY+oaa.maxY; y++)
 			for (int x=oaa.centerX+oaa.minX; x<oaa.centerX+oaa.maxX; x++)
 			{
 				if (oaa.mask.get(orderMaskIndex))
 				{
-					size_t index = (x&map.wMask)+(((y&map.hMask)<<map.wDec));
 					// Update real map
-					map.tiles[index].forbidden |= teamMask;
+					map.addForbidden(x, y, oaa.teamNumber);
 					// Update local map
 					if (oaa.teamNumber == players[localPlayer]->teamNumber)
-						map.displayedForbiddenView.set(index, true);
+						map.displayedForbiddenView.set(map.coordToIndex(x, y), true);
 				}
 				orderMaskIndex++;
 			}
 		teams[oaa.teamNumber]->dirtyGlobalGradient();
 	}
 	else if (oaa.type == BrushTool::MODE_DEL)
 	{
-		Uint32 notTeamMask = ~Team::teamNumberToMask(oaa.teamNumber);
 		size_t orderMaskIndex = 0;
 		for (int y=oaa.centerY+oaa.minY; y<oaa.centerY+oaa.maxY; y++)
 			for (int x=oaa.centerX+oaa.minX; x<oaa.centerX+oaa.maxX; x++)
 			{
 				if (oaa.mask.get(orderMaskIndex))
 				{
-					size_t index = (x&map.wMask)+(((y&map.hMask)<<map.wDec));
 					// Update real map
-					map.tiles[index].forbidden &= notTeamMask;
+					map.removeForbidden(x, y, oaa.teamNumber);
 					// Update local map
 					if (oaa.teamNumber == players[localPlayer]->teamNumber)
-						map.displayedForbiddenView.set(index, false);
+						map.displayedForbiddenView.set(map.coordToIndex(x, y), false);
 				}
 				orderMaskIndex++;
 			}
 
 		// We remove, so we need to refresh the gradients, unfortunately
 		teams[oaa.teamNumber]->dirtyGlobalGradient();
-		map.dirtyBuildingGradients(oaa.centerX+oaa.minX-GRADIENT_DIRTY_BORDER_TILES, oaa.centerY+oaa.minY-GRADIENT_DIRTY_BORDER_TILES, oaa.maxX-oaa.minX+2*GRADIENT_DIRTY_BORDER_TILES, oaa.maxY-oaa.minY+2*GRADIENT_DIRTY_BORDER_TILES, oaa.teamNumber);
 	}
 	else
 		assert(false);
@@ -450,11 +434,7 @@ void Game::executeDelete(const OrderDelete& od)
 		b->launchDelete();
 		assert(b->type);
 		if (b->type->zonableForbidden)
-		{
 			b->owner->dirtyGlobalGradient();
-			int range=b->unitStayRange;
-			map.dirtyBuildingGradients(b->posX-range-GRADIENT_DIRTY_BORDER_TILES, b->posY-range-GRADIENT_DIRTY_BORDER_TILES, 2*GRADIENT_DIRTY_BORDER_TILES+range*2, 2*GRADIENT_DIRTY_BORDER_TILES+range*2, b->owner->teamNumber);
-		}
 	}
 }
 

src/Game_sync.cpp

@@ -43,16 +43,14 @@ void Game::buildProjectSyncStep(Sint32 localTeam)
 		int h=bt->height;
 		if (!map.isHardSpaceForBuilding(posX, posY, w, h))
 		{
-			Uint32 notTeamMask=~Team::teamNumberToMask(teamNumber);
 			for (int y=posY; y<posY+h; y++)
 				for (int x=posX; x<posX+w; x++)
 				{
-					size_t index=(x&map.wMask)+(((y&map.hMask)<<map.wDec));
 					// Update real map
-					map.tiles[index].forbidden&=notTeamMask;
+					map.removeForbidden(x, y, teamNumber);
 					// Update local map
 					if (teamNumber == localTeam)
-						map.displayedForbiddenView.set(index, false);
+						map.displayedForbiddenView.set(map.coordToIndex(x, y), false);
 				}
 			map.updateForbiddenGradient(teamNumber);
 			std::list<BuildProject>::iterator to_erase=bpi;
@@ -65,16 +63,14 @@ void Game::buildProjectSyncStep(Sint32 localTeam)
 			Building *b=addBuilding(posX, posY, typeNum, teamNumber, bpi->unitWorking, bpi->unitWorkingFuture);
 			if (b)
 			{
-				Uint32 notTeamMask=~Team::teamNumberToMask(teamNumber);
 				for (int y=posY; y<posY+h; y++)
 					for (int x=posX; x<posX+w; x++)
 					{
-						size_t index=(x&map.wMask)+(((y&map.hMask)<<map.wDec));
 						// Update real map
-						map.tiles[index].forbidden&=notTeamMask;
+						map.removeForbidden(x, y, teamNumber);
 						// Update local map
 						if (teamNumber == localTeam)
-							map.displayedForbiddenView.set(index, false);
+							map.displayedForbiddenView.set(map.coordToIndex(x, y), false);
 					}
 				map.updateForbiddenGradient(teamNumber);
 				b->owner->addToStaticAbilitiesLists(b);

src/ReplayReader.h

@@ -23,11 +23,12 @@ static constexpr Uint32 REPLAY_MIN_VALID_ORDERS = 5;
 
 //! Oldest replay format (the VERSION_MINOR the replay was written with) that
 //! the reader still accepts. Replays older than this are rejected: versions 90, 92,
-//! 93, 94, 95 and 96 changed the simulation (weighted pathfinding and diagonal timing,
-//! hiring-bucket iteration, trapped-colony elimination, fetch-job apportionment,
-//! round-trip routing and hiring, Echo building-order ids surviving a load), so
-//! earlier replays would diverge from what happened.
-static constexpr Uint16 REPLAY_MINIMUM_VERSION_MINOR = 96;
+//! 93, 94, 95, 96 and 97 changed the simulation (weighted pathfinding and diagonal
+//! timing, hiring-bucket iteration, trapped-colony elimination, fetch-job
+//! apportionment, round-trip routing and hiring, Echo building-order ids surviving a
+//! load, route fields invalidated by the map's topology generation), so earlier
+//! replays would diverge from what happened.
+static constexpr Uint16 REPLAY_MINIMUM_VERSION_MINOR = 97;
 
 /// This class is used for reading replays.
 /// The replay stream is kept open and read every time you do retrieveOrder.

src/SConscript

@@ -624,6 +624,13 @@ if not env['server']:
     regression_test = local.Program('ImmobileUnitGradientHarness', regression_sources)
     local.Alias('immobile-unit-gradient-test', regression_test)
 
+# Real engine regression for building placement and removal refreshing neighbouring route fields, built only by its explicit target.
+if not env['server']:
+    regression_sources = [source for source in source_files if source != 'Glob2.cpp']
+    regression_sources += local.Object('BuildingGradientInvalidationHarness.o', '#test/BuildingGradientInvalidationHarness.cpp')
+    regression_test = local.Program('BuildingGradientInvalidationHarness', regression_sources)
+    local.Alias('building-gradient-invalidation-test', regression_test)
+
 # Real engine regression for resource-fetch target staleness, built only by its explicit target.
 if not env['server']:
     regression_sources = [source for source in source_files if source != 'Glob2.cpp']

src/Version.h

@@ -6,7 +6,7 @@
 // This is the version of map and savegame format, and all of the recorded data on the server
 #define VERSION_MAJOR 0
 #define MINIMUM_VERSION_MINOR 58
-#define VERSION_MINOR 96
+#define VERSION_MINOR 97
 // version 91 saves the live RNG and routing state for deterministic continuation.
 // version 10 adds script saved in game
 // version 11 the gamesfiles do saves which building has been seen under fog of war.
@@ -105,6 +105,9 @@
 // version 96 saves AIEcho::Construction::BuildingOrder::id, which was assigned at
 //            runtime and never serialised, so every pending building order restored
 //            from a save carried an uninitialised heap value as its register key
+// version 97 rebuilds route fields from a map topology generation instead of a proximity
+//            walk, and saves the generation so a loaded game rebuilds on the same ticks:
+//            the simulation changed again
 
 //This must be updated when there are changes to YOG, MapHeader, GameHeader, BasePlayer, BaseTeam,
 //NetMessage, and the likes, in parallel to change of the VERSION_MINOR above

src/building/Building.h

@@ -589,6 +589,9 @@ class Building : public BuildingUtils
 	//! have elapsed since the last rebuild.
 	bool dirtyGradient[SWIM_CLASS_COUNT];
 	Uint32 lastGlobalGradientUpdateStepCounter[SWIM_CLASS_COUNT];
+	//! Map::topologyGeneration when each field was computed. Differs from the
+	//! map's current value exactly when the ground it was built against has moved.
+	Uint32 gradientGeneration[SWIM_CLASS_COUNT];
 	// These flags track physical access (cannot swim / can swim), not travel cost.
 	// All swimming classes share passability, but keep separate weighted fields.
 	//! Last step a unit asked for the gradient; freeIdleGradients drops it when that is long ago.

src/building/Lifecycle.cpp

@@ -156,6 +156,7 @@ void Building::resetPathfindGradients()
 	{
 		delete[] globalGradient[i];
 		globalGradient[i] = NULL;
+		gradientGeneration[i] = 0;
 		for (int r=0; r<MAX_NB_RESOURCES; r++)
 		{
 			delete[] roundTripGradient[r][i];

src/map/Map.cpp

@@ -38,6 +38,7 @@ const int tabClose[8][2]={
 
 Map::Map()
 {
+	topologyGeneration=1;
 	game=NULL;
 
 	arraysBuilt=false;
@@ -93,6 +94,7 @@ Map::~Map(void)
 
 void Map::clear()
 {
+	topologyGeneration=1;
 	// A failed load can own only a subset of these arrays.
 	for (int t=0; t<Team::MAX_COUNT; ++t)
 	{

src/map/Map.h

@@ -344,20 +344,26 @@ class Map
 		tiles[coordToIndex(x, y)].terrain = terrain;
 	}
 
-	void setForbidden(int x, int y, Uint32 forbidden)
-	{
-		tiles[coordToIndex(x, y)].forbidden = forbidden;
-	}
-
+	//! A bump throws away every cached route field in the game, so only paint
+	//! a tile that is not already in the state being asked for.
 	void addForbidden(int x, int y, Uint32 teamNum)
 	{
-		tiles[coordToIndex(x, y)].forbidden |=  Team::teamNumberToMask(teamNum);
+		Tile& c=tiles[coordToIndex(x, y)];
+		const Uint32 mask=Team::teamNumberToMask(teamNum);
+		if ((c.forbidden & mask)==mask)
+			return;
+		c.forbidden |= mask;
+		bumpTopologyGeneration();
 	}
 
 	void removeForbidden(int x, int y, Uint32 teamNum)
 	{
 		Tile& c=tiles[coordToIndex(x, y)];
-		c.forbidden ^= c.forbidden &  Team::teamNumberToMask(teamNum);
+		const Uint32 mask=Team::teamNumberToMask(teamNum);
+		if ((c.forbidden & mask)==0)
+			return;
+		c.forbidden ^= c.forbidden & mask;
+		bumpTopologyGeneration();
 	}
 
 	void addClearArea(int x, int y, Uint32 teamNum)
@@ -518,6 +524,7 @@ class Map
 		for (int yi=y; yi<y+h; yi++)
 			for (int xi=x; xi<x+w; xi++)
 				tiles[coordToIndex(xi, yi)].building = gbid;
+		bumpTopologyGeneration();
 	}
 
 	//! Return the sector index of the sector containing tile (x,y). The
@@ -673,9 +680,14 @@ class Map
 	bool buildingAvailable(Building *building, int swimClass, int x, int y, int *dist);
 	//!requests the next step (dx, dy) to take to get to the building from (x,y)
 	bool pathfindBuilding(Building *building, int swimClass, int x, int y, int *dx, int *dy);
-
-	//! Mark the gradients of this team's buildings in the area for a rebuild. Wrap-safe on x,y
-	void dirtyBuildingGradients(int x, int y, int wl, int hl, int teamNumber);
+
+	//! Bumped whenever a footprint or a forbidden mask changes. A route field
+	//! spans the map, so any such change may cross it: each field records the
+	//! value it was built at and is rebuilt on use once it differs. Resources
+	//! and immobile units are left out on purpose; they change far too often
+	//! and a unit blocked by one forces its own rebuild in pathfindBuilding.
+	Uint32 topologyGeneration;
+	void bumpTopologyGeneration() { topologyGeneration++; }
 	bool pathfindForbidden(const Uint16 *optionGradient, int teamNumber, int swimClass, int x, int y, int *dx, int *dy);
 	enum class AreaKind { Guard, Clear };
 	//! Find the best direction toward a guard or clear area; return true if one has been found.

src/map/gradient/MapGradientBuilding.cpp

@@ -29,6 +29,7 @@ void Map::updateGlobalGradient(Building *building, int swimClass)
 	assert(gradient);
 	building->dirtyGradient[swimClass]=false;
 	building->lastGlobalGradientUpdateStepCounter[swimClass]=game->stepCounter;
+	building->gradientGeneration[swimClass]=topologyGeneration;
 
 	bool isClearingFlag=false;
 	bool isWarFlag=false;