Let any worker stock a completed building (draft: keep the schooling gate for sites only)

.github/workflows/build.yml

@@ -159,6 +159,10 @@ jobs:
           scons -j$(nproc) release=1 server=0 hiring-bucket-test
           python3 test/run-savegame-safety-tests.py --check-preferences build/src/HiringBucketHarness .
 
+      - name: Build and run the hiring level-gate regression
+        run: |
+          scons -j$(nproc) release=1 server=0 level-gate-test
+          python3 test/run-savegame-safety-tests.py --check-preferences build/src/LevelGateHarness .
       - name: Build and run the resource-fetch target regression
         run: |
           scons -j$(nproc) release=1 server=0 resource-fetch-target-test

src/ReplayReader.h

@@ -23,10 +23,13 @@ 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 and 95 changed the simulation (weighted pathfinding and diagonal timing,
+//! 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), so earlier replays would diverge from what happened.
-static constexpr Uint16 REPLAY_MINIMUM_VERSION_MINOR = 95;
+//! 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, one worker level, ungated stocking of completed
+//! buildings), 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

@@ -659,6 +659,13 @@ if not env['server'] and 'hiring-bucket-test' in COMMAND_LINE_TARGETS:
     hiring_sources += local.Object('HiringBucketHarness.o', '#test/HiringBucketHarness.cpp')
     local.Alias('hiring-bucket-test', local.Program('HiringBucketHarness', hiring_sources))
 
+# Which workers a building may hire: completed buildings take any worker,
+# construction and upgrade sites keep the schooling-level gate.
+if not env['server'] and 'level-gate-test' in COMMAND_LINE_TARGETS:
+    level_gate_sources = [source for source in source_files if source != 'Glob2.cpp']
+    level_gate_sources += local.Object('LevelGateHarness.o', '#test/LevelGateHarness.cpp')
+    local.Alias('level-gate-test', local.Program('LevelGateHarness', level_gate_sources))
+
 if not env['server'] and 'custom-setup-test' in COMMAND_LINE_TARGETS:
     custom_sources = [source for source in source_files if source != 'Glob2.cpp']
     custom_sources += local.Object('CustomGameSetupHarness.o', '#test/CustomGameSetupHarness.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 95
+#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.
@@ -102,6 +102,10 @@
 //            rather than refusing it: the simulation changed again
 // version 95 routes and hires by round trip (fetch plus carry) and saves the
 //            round-trip fields with the map runtime state: the simulation changed again
+// version 96 reads one worker level (build) where hiring used to read harvest and
+//            the upgrade menu build, and evens the two out on load
+// version 97 lets any worker stock a completed building; the schooling level only
+//            gates construction and upgrade sites: 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/Misc.cpp

@@ -140,8 +140,13 @@ bool Building::canUnitWorkHere(Unit* unit)
 	}
 	else if(unit->typeNum ==  WORKER)
 	{
-		int actLevel=unit->level[HARVEST];
-		if(type->level <= actLevel)
+		// The level gate is for builders: raising or upgrading a level-N
+		// building takes a worker schooled to level N. A completed building
+		// only wants stock carried to it (wheat and fruit for an inn, stone
+		// for a tower), and any worker can carry.
+		if(!type->isBuildingSite)
+			return true;
+		if(type->level <= unit->workerLevel())
 			return true;
 	}
 	return false;

src/building/Step.cpp

@@ -33,7 +33,7 @@ namespace
 	/// higher is preferred.
 	int bringResourcesLevel(const Unit* unit)
 	{
-		return unit->level[HARVEST] * HARVEST_LEVEL_WEIGHT + unit->level[WALK];
+		return unit->workerLevel() * HARVEST_LEVEL_WEIGHT + unit->level[WALK];
 	}
 }
 
@@ -538,7 +538,7 @@ bool Building::subscribeForFlagingStep()
 					int hp=(unit->hp<<4)/unit->race->unitTypes[0][0].performance[HP];
 					int dist = distances[n];
 					int value=dist-timeLeft-hp;
-					int level = unit->level[HARVEST];
+					int level = unit->workerLevel();
 					//We want to minimize the level of harvesting units, so that the higher level
 					//units are available for more important work.
 					if ((level < minLevel) || (level==minLevel && value<minValue))

src/gui/GameGUIDrawUnitInfos.cpp

@@ -176,10 +176,6 @@ void GameGUI::drawUnitInfos(void)
 		drawAbilityRow(rowX, ypos, "[Build]", 1 + selUnit->level[BUILD], selUnit->performance[BUILD]);
 	ypos += YOFFSET_TEXT_LINE;
 
-	if (selUnit->performance[HARVEST])
-		drawAbilityRow(rowX, ypos, "[Harvest]", 1 + selUnit->level[HARVEST], selUnit->performance[HARVEST]);
-	ypos += YOFFSET_TEXT_LINE;
-
 	if (selUnit->performance[ATTACK_SPEED])
 		drawAbilityRow(rowX, ypos, "[At. speed]", 1 + selUnit->level[ATTACK_SPEED], selUnit->performance[ATTACK_SPEED]);
 	ypos += YOFFSET_TEXT_LINE;

src/map/edit/MapEdit.h

@@ -591,8 +591,6 @@ class MapEdit
 	ValueScrollBox* unitWalkLevelScrollBox;
 	FractionValueText* unitSwimLevelLabel;
 	ValueScrollBox* unitSwimLevelScrollBox;
-	FractionValueText* unitHarvestLevelLabel;
-	ValueScrollBox* unitHarvestLevelScrollBox;
 	FractionValueText* unitBuildLevelLabel;
 	ValueScrollBox* unitBuildLevelScrollBox;
 	FractionValueText* unitAttackSpeedLevelLabel;

src/map/edit/MapEditActionUnit.cpp

@@ -107,8 +107,6 @@ bool MapEdit::performUnitAction(const std::string& action, int relMouseX, int re
 			unitWalkLevelScrollBox->setValues(&view.selectedUnit->level[WALK]);
 			unitSwimLevelLabel->setValues(&view.selectedUnit->level[SWIM]);
 			unitSwimLevelScrollBox->setValues(&view.selectedUnit->level[SWIM]);
-			unitHarvestLevelLabel->setValues(&view.selectedUnit->level[HARVEST]);
-			unitHarvestLevelScrollBox->setValues(&view.selectedUnit->level[HARVEST]);
 			unitBuildLevelLabel->setValues(&view.selectedUnit->level[BUILD]);
 			unitBuildLevelScrollBox->setValues(&view.selectedUnit->level[BUILD]);
 			unitAttackSpeedLevelLabel->setValues(&view.selectedUnit->level[ATTACK_SPEED]);
@@ -128,11 +126,6 @@ bool MapEdit::performUnitAction(const std::string& action, int relMouseX, int re
 				unitSwimLevelLabel->disable();
 				unitSwimLevelScrollBox->disable();
 			}
-			if(!view.selectedUnit->canLearn[HARVEST])
-			{
-				unitHarvestLevelLabel->disable();
-				unitHarvestLevelScrollBox->disable();
-			}
 			if(!view.selectedUnit->canLearn[BUILD])
 			{
 				unitBuildLevelLabel->disable();
@@ -163,13 +156,12 @@ bool MapEdit::performUnitAction(const std::string& action, int relMouseX, int re
 	{
 		refreshSelectedUnitPerformance(SWIM);
 	}
-	else if(action=="update unit harvest level")
-	{
-		refreshSelectedUnitPerformance(HARVEST);
-	}
 	else if(action=="update unit build level")
 	{
-		refreshSelectedUnitPerformance(BUILD);
+		// One worker level: the build box drives harvest as well.
+		Unit* u=game.teams[Unit::GIDtoTeam(selectedUnitGID)]->myUnits[Unit::GIDtoID(selectedUnitGID)];
+		u->setWorkerLevel(u->level[BUILD]);
+		hasMapBeenModified = true;
 	}
 	else if(action=="update unit attack speed level")
 	{

src/map/edit/MapEditCtor.cpp

@@ -174,10 +174,8 @@ MapEdit::MapEdit()
 	unitWalkLevelScrollBox = new ValueScrollBox(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 300, 112, 16), "unit editor", "unit editor walk level scroll box", "update unit walk level", NULL, 3);
 	unitSwimLevelLabel = new FractionValueText(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 316, 128, 16), "unit editor", "unit editor swim level label", "", "[Swim]", NULL, 3);
 	unitSwimLevelScrollBox = new ValueScrollBox(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 332, 112, 16), "unit editor", "unit editor swim level scroll box", "update unit swim level", NULL, 3);
-	unitHarvestLevelLabel = new FractionValueText(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 348, 128, 16), "unit editor", "unit editor harvest level label", "", "[Harvest]", NULL, 3);
-	unitHarvestLevelScrollBox = new ValueScrollBox(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 364, 112, 16), "unit editor", "unit editor harvest level scroll box", "update unit harvest level", NULL, 3);
-	unitBuildLevelLabel = new FractionValueText(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 380, 128, 16), "unit editor", "unit editor build level label", "", "[Build]", NULL, 3);
-	unitBuildLevelScrollBox = new ValueScrollBox(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 396, 112, 16), "unit editor", "unit editor build level scroll box", "update unit build level", NULL, 3);
+	unitBuildLevelLabel = new FractionValueText(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 348, 128, 16), "unit editor", "unit editor build level label", "", "[Build]", NULL, 3);
+	unitBuildLevelScrollBox = new ValueScrollBox(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 364, 112, 16), "unit editor", "unit editor build level scroll box", "update unit build level", NULL, 3);
 	unitAttackSpeedLevelLabel = new FractionValueText(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 348, 128, 16), "unit editor", "unit editor attack speed level label", "", "[At. speed]", NULL, 3);
 	unitAttackSpeedLevelScrollBox = new ValueScrollBox(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 364, 112, 16), "unit editor", "unit editor attack speed level scroll box", "update unit attack speed level", NULL, 3);
 	unitAttackStrengthLevelLabel = new FractionValueText(*this, widgetRectangle(globalContainer->gfx->getW()-RIGHT_MENU_WIDTH+8+decX, 380, 128, 16), "unit editor", "unit editor attack strength level label", "", "[At. strength]", NULL, 3);
@@ -192,8 +190,6 @@ MapEdit::MapEdit()
 	addWidget(unitWalkLevelScrollBox);
 	addWidget(unitSwimLevelLabel);
 	addWidget(unitSwimLevelScrollBox);
-	addWidget(unitHarvestLevelLabel);
-	addWidget(unitHarvestLevelScrollBox);
 	addWidget(unitBuildLevelLabel);
 	addWidget(unitBuildLevelScrollBox);
 	addWidget(unitAttackSpeedLevelLabel);

src/team/TeamRouting.cpp

@@ -252,7 +252,7 @@ int Team::maxBuildLevel(void)
 		Unit *u=myUnits[i];
 		if (u && u->performance[BUILD])
 		{
-			int unitLevel=u->level[BUILD];
+			int unitLevel=u->workerLevel();
 			if (unitLevel>maxLevel)
 				maxLevel=unitLevel;
 		}

src/unit/Unit.cpp

@@ -324,3 +324,12 @@ void Unit::syncStep(void)
 	if (magicActionAnimation > 0)
 		magicActionAnimation--;
 }
+
+void Unit::setWorkerLevel(Sint32 newLevel)
+{
+	for (int ability : {(int)BUILD, (int)HARVEST})
+	{
+		level[ability] = newLevel;
+		performance[ability] = race->getUnitType(typeNum, newLevel)->performance[ability];
+	}
+}

src/unit/Unit.h

@@ -260,6 +260,12 @@ class Unit : public UnitUtils
 	//! Pathfinding swim class from the unit's walk and swim speeds (see Map::swimClass).
 	int swimClass() const;
 	Sint32 level[NB_ABILITY];
+	//! The worker's schooling level. Harvest and build are taught together by
+	//! the school and mean one thing in play, which building tier the worker
+	//! may raise, so they are kept equal and read through here.
+	Sint32 workerLevel() const { return level[BUILD]; }
+	//! Set both halves of the worker level, and their performance, together.
+	void setWorkerLevel(Sint32 newLevel);
 	bool canLearn[NB_ABILITY];
 	Sint32 experience;
 	Sint32 experienceLevel;

src/unit/UnitDisplacement.cpp

@@ -342,9 +342,14 @@ void Unit::handleDisplacement(void)
 						else
 						{
 							assert(canLearn[destinationPurpose]);
-							level[destinationPurpose] = attachedBuilding->type->level + 1;
-							UnitType *ut = race->getUnitType(typeNum, level[destinationPurpose]);
-							performance[destinationPurpose] = ut->performance[destinationPurpose];
+							if (destinationPurpose == BUILD || destinationPurpose == HARVEST)
+								setWorkerLevel(attachedBuilding->type->level + 1);
+							else
+							{
+								level[destinationPurpose] = attachedBuilding->type->level + 1;
+								UnitType *ut = race->getUnitType(typeNum, level[destinationPurpose]);
+								performance[destinationPurpose] = ut->performance[destinationPurpose];
+							}
 						}
 
 

src/unit/UnitSerialization.cpp

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 // Copyright (C) 2001-2004 Stephane Magnenat & Luc-Olivier de Charrière
 
+#include <algorithm>
 #include "Unit.h"
 #include "Race.h"
 #include "Team.h"
@@ -86,6 +87,10 @@ void Unit::load(GAGCore::InputStream *stream, Team *owner, Sint32 versionMinor)
 		stream->readLeaveSection();
 	}
 	stream->readLeaveSection();
+	// Harvest and build are one worker level; a map edited with the two apart
+	// (the editor used to offer them separately) keeps the higher one.
+	if (canLearn[BUILD] && level[HARVEST] != level[BUILD])
+		setWorkerLevel(std::max(level[HARVEST], level[BUILD]));
 
 
 	experience = stream->readSint32("experience");

test/LevelGateHarness.cpp

@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+// Real-engine regression for Building::canUnitWorkHere: a completed building
+// hires any worker to stock it, a construction or upgrade site keeps the
+// schooling-level gate.
+#define SDL_MAIN_HANDLED
+#ifdef main
+#undef main
+#endif
+#include "GlobalContainer.h"
+#include "FileManager.h"
+#include <SDL.h>
+#include <string>
+#include "Game.h"
+#include "GameGUI.h"
+#include "Unit.h"
+#include "Team.h"
+#include "MapInternal.h"
+#include "Race.h"
+#include "Ressource.h"
+#include "IntBuildingType.h"
+#include <cstdio>
+#include <cstdlib>
+
+GlobalContainer* globalContainer = nullptr;
+
+static void require(bool ok, const char* message)
+{
+	if (!ok) { std::fprintf(stderr, "FAIL: %s\n", message); std::exit(1); }
+}
+
+// One hiring pass: a worker of the given schooling level, carrying `carried`,
+// four tiles from a building of the given kind that wants it. Returns whether
+// the building hired the worker; `tooLowLevel` reports the rejection counter.
+static bool hiringPass(const char* kind, int level, bool site, int carried, int workerLevel, Uint32* tooLowLevel, int harvestLevel = -1)
+{
+	GameGUI gui;
+	Game& game = gui.game;
+	game.map.setSize(5, 5, GRASS);
+	game.map.setGame(&game);
+	game.addTeam(0);
+	Team* team = game.teams[0];
+	team->race.loadDefault();
+	int typeNum = globalContainer->buildingsTypes.getTypeNum(kind, level, site);
+	require(typeNum >= 0, "building type exists");
+	Building* building = game.addBuilding(8, 8, typeNum, 0);
+	require(building != nullptr, "create building");
+	building->maxUnitWorking = 2;
+	building->resources[carried] = 0;
+	building->updateCallLists();
+	Unit* unit = game.addUnit(12, 12, 0, WORKER, 0, 255, 0, 0);
+	require(unit != nullptr, "create worker");
+	unit->level[HARVEST] = harvestLevel < 0 ? workerLevel : harvestLevel;
+	unit->level[BUILD] = workerLevel;
+	unit->carriedResource = carried;
+	unit->activity = Unit::ACT_RANDOM;
+	unit->medical = Unit::MED_FREE;
+	team->updateAllBuildingTasks();
+	*tooLowLevel = building->unitsFailingRequirements[Building::UnitTooLowLevel];
+	bool hired = building->unitsWorking.size() == 1;
+	std::printf("%s level %d %s, worker level %d -> %s (too low level: %u)\n",
+		kind, level + 1, site ? "site" : "completed", workerLevel, hired ? "hired" : "refused", *tooLowLevel);
+	return hired;
+}
+
+int main(int argc, char** argv)
+{
+	SDL_SetMainReady();
+	require(argc == 3, "usage: harness PROFILE ROOT");
+	require(std::string(argv[1]).find("glob2-save-test-") == 0, "disposable profile required");
+	GlobalContainer globals(argv[1]);
+	globals.fileManager->addDir(argv[2]);
+	globalContainer = &globals;
+	globals.runNoX = true;
+	globals.settings.rememberUnit = false;
+	globals.buildingsTypes.init();
+	IntBuildingType::init();
+	Race::loadDefault();
+
+	Uint32 tooLow = 0;
+	// Completed buildings: any worker stocks them.
+	require(hiringPass("inn", 1, false, CORN, 0, &tooLow) && tooLow == 0, "unschooled worker stocks a level-2 inn");
+	require(hiringPass("inn", 2, false, CORN, 0, &tooLow) && tooLow == 0, "unschooled worker stocks a level-3 inn");
+	require(hiringPass("defencetower", 2, false, STONE, 0, &tooLow) && tooLow == 0, "unschooled worker stocks a level-3 tower");
+	// Sites: the schooling level still gates who builds.
+	require(!hiringPass("inn", 1, true, WOOD, 0, &tooLow) && tooLow == 1, "unschooled worker refused by a level-2 inn site");
+	require(hiringPass("inn", 1, true, WOOD, 1, &tooLow) && tooLow == 0, "level-1 worker builds a level-2 inn site");
+	require(!hiringPass("inn", 2, true, WOOD, 1, &tooLow) && tooLow == 1, "level-1 worker refused by a level-3 inn site");
+	require(hiringPass("inn", 2, true, WOOD, 2, &tooLow) && tooLow == 0, "level-2 worker builds a level-3 inn site");
+	// The gate reads the worker level (build); a stale harvest value does not matter.
+	require(hiringPass("inn", 1, true, WOOD, 1, &tooLow, 0) && tooLow == 0, "build level 1 with harvest 0 builds a level-2 inn site");
+	std::puts("PASS completed buildings hire any worker, sites keep the level gate");
+	return 0;
+}