fixed emergency protocol by removing sheduled wares requested before ... by tost11 · Pull Request #1961 · Return-To-The-Roots/s25client

tost11 · 2026-07-16T13:41:54Z

When emergency protocol is activated and many buildings are already planned, the wares already scheduled for delivery are ignoring the protocol.

So now when emergency protocol is activated, those wares get removed (if not for the sawmill or woodcutter).

This also solved the issue for AI enemies on very low resources where they're not able to start because the first sawmill never got built.

…activation

…wares

… only on activation)

Flamefire

LGTM,
trivial style issue in the NULL check which I fixed directly. git pull if you need to do further changes

Flamefire

You need to run clang-format first. If you have it installed when configuring you can run e.g. make clangFormat
Or use the diff as shown in the CI job.

Can you add a test for that? Might be a bit tricky though

Spikeone · 2026-07-19T08:31:43Z

Well, this is original behavior - you were also able to store out wares to get them going anway (a tactic used with very low goods frequently). This fix breaks this original behavior, doesn't it?

This sounds reading your description more like a problem the AI has, and should be fixed for the AI instead of for players (maybe the AI can use that system which is slight cheating but I'd opt for a smarter AI instead of an AI playing only by rules)

Flamefire · 2026-07-19T11:08:18Z

Well, this is original behavior - you were also able to store out wares to get them going anway (a tactic used with very low goods frequently). This fix breaks this original behavior, doesn't it?

What we have:

When emergency program is active newly built buildings don't get wares except woodcutter or sawmills
With this: Additionally wares ordered for such "blocked" buildings which are still in a warehouse, i.e. not carried out yet, are stopped, wares already on the way are not.

Not fully sure how the original handled that as it seems like a minor corner case happening rarely.
If in the original wares for other buildings are still carried out, then I'd also say that we guard this behavior for AI only. If it helps then I don't see an issue there as it is such a minimal QoL change for rare cases that it likely won't even be noticed in most cases.

tost11 · 2026-07-28T15:38:01Z

The issue can also occur for the player, like On low resources, start when starting with multiple buildings (but the sawmill first, but on hilly terrain) and then multiple other buildings (fast before all boards are given out). Then all boards are scheduled for building. The sawmill and woodcutter are already planed. Then some time later the Terain Blow sawmill is leveled, and it requests boards. But all boards are already given out to other buildings. That results in no boards for the sawmill.

But yes, it is an edge case, and yes, I mainly built it for the AI to start proper play on some maps where it gets stuck on low-resource starts.

But actually it's an issue in the game, not only an AI issue.

So what now? I see multiple options that could be implemented to not block fast start for players.

Fix it only on AI, but behavior stays broken on edge cases.
Check what the behavior was in the original game.
Improve emergency protocol with:

-- Only hold back needed boards (2 for the sawmill and 2 for the woodcutter) depending on what buildings are missing. Also maybe 1 more if the street gets removed and one is lost by the player or AI.
-- Unlock emergency protocol if the boards reach the building, not when finished (but on the other hand, without those building boards, production isn't possible anyway).
-- Map boards to exactly one needed building; for example, if two woodcutters build a target, one gets the boards. so other boards can be normally used.

What do you prefer?

For me it would be the last option, and I also would implement that more complicated but better emergency program behavior if wanted.

Flamefire · 2026-07-29T07:57:52Z

But yes, it is an edge case, and yes, I mainly built it for the AI to start proper play on some maps where it gets stuck on low-resource starts.

But actually it's an issue in the game, not only an AI issue.

That depends on the definition of "issue". We want (at least the default) behavior to match that of S2, so if "emergency mode" there meant that only newly built buildings do not get boards/stones then that is (usually) what we want.
AI may "cheat" and that's fine as a player could invoke this behavior by removing roads, so that's a shortcut.

So what now? I see multiple options that could be implemented to not block fast start for players.
* Fix it only on AI, but behavior stays broken on edge cases.

No issue with that in any case

* Check what the behavior was in the original game.

See above: That would be the most important thing to check first although it may be difficult to get into exactly this situation. But it looks like your description above should be reasonably easy to check. Could you do that?

* Improve emergency protocol with:

I think that is too complicated to avoid such a rare issue. In the end this is a resource management game and the emergency mode already a QoL improvement only. So it could be "incomplete" leaving more responsibility to the player.

However as queued wares do not turn up for the player anywhere in the UI it might seem like emergency mode is ignored for a short time. I didn't fully understand @Spikeone

Well, this is original behavior - you were also able to store out wares to get them going anway (a tactic used with very low goods frequently). This fix breaks this original behavior, doesn't it?

What exactly does this break? You build a building, wares for this building are queued, emergency mode activates, wares for building stop being carried out as-if the building was built/connected right after the activation.
If you can "force" delivering wares in emergency mode by out-storing then this isn't blocked by this change. Not sure if it was possible in RttR before, but if it was it still will be.

So I'd lean into accepting this change to enforce the visible semantics: When emergency mode is active no ware will leave the WHs (with the purpose) of going to a "disallowed" building site.
Or what do you thing @Spikeone ?

tost11 · 2026-08-02T18:52:39Z

I have tested the behavior in the Siedler 2 Gold Edition.

It seems to be the cleverest approach. Boards are stored until only 4 are left. If a woodcutter builds it, it gets two boards. Another one would not receive any boards (like it's currently here); only the sawmill gets them (see screenshots).

When both buildings are finished, the protocol is deactivated.

I have also checked the emergency protocol for stones; if you run out of stones, you are doomed—no emergency protocol for it.

Flamefire

I verified with the original too in various situations and you are right: They are immediately stopped in warehouses.

We do have a difference though: S2 activates at 4 boards, RTTR at 10.

Anyway, some minor polishing here and it's good to go

Flamefire · 2026-08-03T11:29:45Z

+    for(auto it = ware_list.begin(); it != ware_list.end();)
+    {
+        Ware* ware = *it;
+        // checks if this ware is


incomplete comment?

Flamefire · 2026-08-03T11:32:32Z

    const Statistic& GetStatistic(StatisticTime time) const { return statistic[time]; };
    unsigned GetStatisticCurrentValue(StatisticType idx) const { return statisticCurrentData[idx]; }

+    // remove all wares that are already scheduled but ignoring emergency protocol


Not sure what "ignoring ..." means here. Maybe:

Suggested change

// remove all wares that are already scheduled but ignoring emergency protocol

// Stop wares restricted in emergency mode that are waiting in warehouse to be transported already

Flamefire · 2026-08-03T11:34:52Z

+
+BOOST_FIXTURE_TEST_CASE(EmergencyProtoclActiveWoodcutterAndSawmillCanBuild, EmergencyFixture)
+{
+    initGameRNG();


move to fixture

Flamefire · 2026-08-03T11:36:00Z

+
+struct EmergencyFixture : public WorldFixture<CreateEmptyWorld, 1>
+{
+    nobHQ* HQ = world.GetPlayer(0).GetHQ();


Suggested change

nobHQ* HQ = world.GetPlayer(0).GetHQ();

nobHQ* hq = world.GetPlayer(0).GetHQ();

Flamefire · 2026-08-03T11:40:55Z

+
+        pos = world.GetPlayer(0).GetHQPos() + MapPoint(3, 0);
+        world.SetBuildingSite(BuildingType::Farm, pos, 0);
+        world.BuildRoad(0, false, world.GetNeighbour(pos, Direction::SouthEast),


Maybe derive from WorldWithGCExecution and use BuildRoadForBlds which checks the the roads are built. Especially in the tests it is otherwise not fully clear that the building site is actually connected. Here it is reasonably clear, but for GetHQPos() + MapPoint(-1, 2) it is a bit trickier

Flamefire · 2026-08-03T11:44:25Z

+        RTTR_EXEC_TILL(500, HQ->GetInventory()[GoodType::Boards] == 10);
+
+        // activate program (with 10 boards it should trigger)
+        world.GetPlayer(0).TestForEmergencyProgramm();


Can't we test that this is done automatically? I.e. shouldn't the above activate it? Or at least in the next GF

Flamefire · 2026-08-03T11:45:05Z

+        // wait for some more ticks to give time if not working to deliver more boards
+        RTTR_SKIP_GFS(200);
+        // check boards are still fine and protocol working
+        BOOST_TEST_CHECK(world.GetPlayer(0).GetHQ()->GetInventory()[GoodType::Boards] == 10);


Suggested change

// wait for some more ticks to give time if not working to deliver more boards

RTTR_SKIP_GFS(200);

// check boards are still fine and protocol working

BOOST_TEST_CHECK(world.GetPlayer(0).GetHQ()->GetInventory()[GoodType::Boards] == 10);

// No more boards are carried out to the farms due to emergency protocol

RTTR_SKIP_GFS(200);

BOOST_TEST_CHECK(hq->GetInventory()[GoodType::Boards] == 10);

Flamefire · 2026-08-03T11:45:26Z

+    }
+};
+
+BOOST_FIXTURE_TEST_CASE(EmergencyProtoclActiveWoodcutterAndSawmillCanBuild, EmergencyFixture)


Typo here and below

Suggested change

BOOST_FIXTURE_TEST_CASE(EmergencyProtoclActiveWoodcutterAndSawmillCanBuild, EmergencyFixture)

BOOST_FIXTURE_TEST_CASE(EmergencyProtocolActiveWoodcutterAndSawmillCanBuild, EmergencyFixture)

Flamefire · 2026-08-03T11:46:49Z

+    // check if inventory boards are given out
+    RTTR_EXEC_TILL(200, HQ->GetInventory()[GoodType::Boards] < 10);
+
+    // check if building where found


Suggested change

// check if building where found

// check that buildings are built

Flamefire · 2026-08-03T11:47:47Z

+    // wait for some more ticks to give time if not working to deliver more boards
+    RTTR_SKIP_GFS(500);
+    // check boards are still fine and protocol working
+    BOOST_TEST_CHECK(world.GetPlayer(0).GetHQ()->GetInventory()[GoodType::Boards] == 10);


Suggested change

// wait for some more ticks to give time if not working to deliver more boards

RTTR_SKIP_GFS(500);

// check boards are still fine and protocol working

BOOST_TEST_CHECK(world.GetPlayer(0).GetHQ()->GetInventory()[GoodType::Boards] == 10);

// No boards are carried out to the farms or watchtower due to emergency protocol

RTTR_SKIP_GFS(500);

BOOST_TEST_CHECK(hq->GetInventory()[GoodType::Boards] == 10);

lukas added 2 commits July 16, 2026 15:33

fixed emergency protocol by removing sheduled wares requested before …

89c524a

…activation

added type = boards for additinal condigion to remove from emergency …

0e13b2c

…wares

Flamefire requested changes Jul 16, 2026

View reviewed changes

Comment thread libs/s25main/GamePlayer.cpp Outdated

Comment thread libs/s25main/GamePlayer.h Outdated

Comment thread libs/s25main/GamePlayer.cpp Outdated

pr improvements (comments, extra check emergency ware function, check…

97f5b63

… only on activation)

Flamefire requested changes Jul 17, 2026

View reviewed changes

Comment thread libs/s25main/GamePlayer.h Outdated

Comment thread libs/s25main/GamePlayer.cpp Outdated

pr-improvements (nullcheck, static function)

324d8ed

Flamefire reviewed Jul 17, 2026

View reviewed changes

Comment thread libs/s25main/GamePlayer.cpp Outdated

Remove comparison to nullptr

9dac609

Flamefire approved these changes Jul 17, 2026

View reviewed changes

Flamefire enabled auto-merge July 17, 2026 10:29

Flamefire requested changes Jul 17, 2026

View reviewed changes

lukas added 2 commits July 17, 2026 17:26

clang formater

2b5735f

added tests for emergency protocol

0fe2d3c

auto-merge was automatically disabled July 17, 2026 22:53
Head branch was pushed to by a user without write access

fixed gameframe condition timeouts on emergency protocol tests

689112e

tost11 force-pushed the feature/fix-emergency-protocol branch from bc104e8 to 689112e Compare July 17, 2026 23:02

Flamefire previously approved these changes Jul 18, 2026

View reviewed changes

formated test file

0c79309

tost11 dismissed Flamefire’s stale review via 0c79309 July 18, 2026 12:31

Flamefire requested changes Aug 3, 2026

View reviewed changes

	// remove all wares that are already scheduled but ignoring emergency protocol
	// Stop wares restricted in emergency mode that are waiting in warehouse to be transported already

	nobHQ* HQ = world.GetPlayer(0).GetHQ();
	nobHQ* hq = world.GetPlayer(0).GetHQ();

	BOOST_FIXTURE_TEST_CASE(EmergencyProtoclActiveWoodcutterAndSawmillCanBuild, EmergencyFixture)
	BOOST_FIXTURE_TEST_CASE(EmergencyProtocolActiveWoodcutterAndSawmillCanBuild, EmergencyFixture)

	// check if building where found
	// check that buildings are built

Uh oh!

Conversation

tost11 commented Jul 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Flamefire left a comment

Choose a reason for hiding this comment

Uh oh!

Flamefire left a comment

Choose a reason for hiding this comment

Uh oh!

Spikeone commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Flamefire commented Jul 19, 2026

Uh oh!

tost11 commented Jul 28, 2026

Uh oh!

Flamefire commented Jul 29, 2026

Uh oh!

tost11 commented Aug 2, 2026

Uh oh!

Flamefire left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Spikeone commented Jul 19, 2026 •

edited

Loading