Configurable mine resource behavior

[DevOpsOfChaos]

Summary

This PR implements configurable mine resource behavior as a cleaned-up replacement path for the broader #1501 direction.

It adds per-mine resource behavior settings for coal, iron, gold, and granite mines, while reusing existing inexhaustible mine addon IDs where appropriate instead of stacking additional addons on top.

Details

• Add configurable mine resource behavior per mine type:
  • Default
  • S4-like exhaustion
  • Inexhaustible
  • Work everywhere
• Reuse existing INEXHAUSTIBLE_* addon IDs as per-mine behavior list settings where possible
• Map old boolean ON values to the new Inexhaustible behavior
• Migrate legacy global INEXHAUSTIBLE_MINES settings to per-mine inexhaustible behavior when loading old settings
• Remove the separate granite work-everywhere addon path
• Treat Work everywhere as ignoring ground-resource requirements instead of creating resources in the world
• Add configurable no-output fallback behavior for failed S4-like production cycles
• Teach AI resource rating and mine planning about configured mine behavior
• Adjust displayed mine productivity for S4-like resource chance
• Add integration coverage for production behavior, AI behavior, serialization/migration, and productivity display

S4-like behavior

For S4-like behavior:

• each mine type can be configured independently
• a mine attempts production based on remaining matching resources in its mining radius
• 20 remaining matching resources is the explicit full-productivity reference
• below that reference, production chance is reduced proportionally
• successful production depletes resources down to 1, not to 0
• failed production runs a no-output cycle
• the no-output cycle can either produce nothing or use the configured fallback production behavior

Work-everywhere behavior

Work everywhere means that the mine skips the ground-resource requirement.

It does not create temporary resources in the world and does not deplete such generated resources.

Validation

• clang-format version 10.0.0
• Built Test_integration
• Built Test_UI
• Passed Test_integration --run_test=Production
• Passed Test_integration --run_test=AI
• Passed Test_integration --run_test=Serialization
• Passed ctest --test-dir .\build-vs-x64-debug-local -C Debug -R "^Test_integration$" --output-on-failure
• Passed Test_UI.exe --report_level=short
• git diff --check upstream/master...HEAD passed

[Flamefire]

Thanks for taking care of that.
Please remove duplications and check back with what was discussed before. This includes:

• There is no need for multiple addons for a single mine type: GRANITEMINES_WORK_EVERYWHERE is superflous and all the existing "inexhaustible*" addons should be integrated into this ones and use the same IDs: The OFF setting should be "Default" and ON (the new) "inexhaustible"
• FindPointWithResourceQuiet duplicates code already present -> Reuse existing one, can be done by simply adding a default bool parameter notify
• INEXHAUSTIBLE_MINES addon should be removed: When detected during loading it should set the new addons accordingly, keep ID for that with a TODO for gamedata version increasing
• Explain CalcAverageDisplayProductivity please, why the extra methods?
• Isn't there already a method to convert AIResource to Resource so GetMineBuildingType doesn't need to be (fully) implemented twice?
• CanCreateWorkEverywhereResource looks odd: "Work everywhere" should be just that: Ignore any resource in the ground, not add resources to the world, doesn't it? So just skip the search
• IsMineResourceDepletable shouldn't use GetConfiguredMineResourceBehavior, in fact it seems only a single function is required, i.e. both Get... combined.

Maybe more.

Please give a quick summary of how "s4 like behavior" is supposed to work, so @Spikeone can verify it is what he intended in #1501 and we can verify it is implemented according to that spec.

[DevOpsOfChaos]

Thanks for taking care of that. Please remove duplications and check back with what was discussed before. This includes:

• There is no need for multiple addons for a single mine type: GRANITEMINES_WORK_EVERYWHERE is superflous and all the existing "inexhaustible*" addons should be integrated into this ones and use the same IDs: The OFF setting should be "Default" and ON (the new) "inexhaustible"
• FindPointWithResourceQuiet duplicates code already present -> Reuse existing one, can be done by simply adding a default bool parameter notify
• INEXHAUSTIBLE_MINES addon should be removed: When detected during loading it should set the new addons accordingly, keep ID for that with a TODO for gamedata version increasing
• Explain CalcAverageDisplayProductivity please, why the extra methods?
• Isn't there already a method to convert AIResource to Resource so GetMineBuildingType doesn't need to be (fully) implemented twice?
• CanCreateWorkEverywhereResource looks odd: "Work everywhere" should be just that: Ignore any resource in the ground, not add resources to the world, doesn't it? So just skip the search
• IsMineResourceDepletable shouldn't use GetConfiguredMineResourceBehavior, in fact it seems only a single function is required, i.e. both Get... combined.

Maybe more.

Please give a quick summary of how "s4 like behavior" is supposed to work, so @Spikeone can verify it is what he intended in #1501 and we can verify it is implemented according to that spec.

Thanks, that direction makes sense.

My understanding of the requested cleanup is:

• fold the existing INEXHAUSTIBLE_* addon IDs into the new per-mine behavior list settings
• keep OFF == Default and map the old ON value to the new Inexhaustible behavior
• remove the extra granite-specific work-everywhere addon path
• remove the old global INEXHAUSTIBLE_MINES behavior as an active setting and migrate it during loading to the new per-mine settings, keeping its ID only for compatibility/TODO until the gamedata version can be increased
• avoid helper duplication such as FindPointWithResourceQuiet
• treat WorkEverywhere as “ignore ground resources”, not “create resources in the world”
• simplify the effective behavior/depletion helper logic

My current S4-like behavior implementation is intended to work like this:

• each mine type can be configured independently
• for S4-like behavior, a mine can attempt production based on the remaining matching resources in its mining radius
• the production chance is derived from remaining resource amount compared to the relevant possible resource capacity around the mine
• resources are depleted only down to 1, not to 0
• if the chance check fails, the mine runs a no-output cycle
• the no-output cycle can either produce nothing or use the configured fallback production behavior

I’ll rework the implementation toward the existing-ID/list-setting approach and remove the duplicated/intermediate pieces instead of stacking this on top of the old addon set.

[Flamefire]

• for S4-like behavior, a mine can attempt production based on the remaining matching resources in its mining radius

• the production chance is derived from remaining resource amount compared to the relevant possible resource capacity around the mine

• resources are depleted only down to 1, not to 0

• if the chance check fails, the mine runs a no-output cycle

• the no-output cycle can either produce nothing or use the configured fallback production behavior

@Spikeone Looks correct to me. Can you confirm?

"possible resource capacity" isn't fully clear to me. There are 7 points considered and currently each resource gives 5% chance. I.e. when all have at least 3 resources we will have 100%. IIRC the max amount of resources per node is 7 (we should have a constant for that if we don't already have one)
The code should be clear here, the 5% looks random. The chance could be e.g. directly calculated from the points and use the number of points and max amount. Or the constant derived from something like that.

[DevOpsOfChaos]

• for S4-like behavior, a mine can attempt production based on the remaining matching resources in its mining radius
• the production chance is derived from remaining resource amount compared to the relevant possible resource capacity around the mine
• resources are depleted only down to 1, not to 0
• if the chance check fails, the mine runs a no-output cycle
• the no-output cycle can either produce nothing or use the configured fallback production behavior

@Spikeone Looks correct to me. Can you confirm?

"possible resource capacity" isn't fully clear to me. There are 7 points considered and currently each resource gives 5% chance. I.e. when all have at least 3 resources we will have 100%. IIRC the max amount of resources per node is 7 (we should have a constant for that if we don't already have one) The code should be clear here, the 5% looks random. The chance could be e.g. directly calculated from the points and use the number of points and max amount. Or the constant derived from something like that.

Good point. The current implementation intentionally models the existing S4-like “full productivity around ~20 remaining resources” behavior rather than deriving 100% from the absolute theoretical map capacity.

The current 5% per remaining resource effectively means:

• 20 remaining matching resources => 100% production chance
• depletion continues reducing the chance below that point

I agree the current formulation makes the 5% look arbitrary in code and in the PR description.

I will rework this to express the behavior through named constants / derived calculation so the intended reference point becomes explicit instead of looking like a magic number.

[Flamefire]

Good point. The current implementation intentionally models the existing S4-like “full productivity around ~20 remaining resources” behavior rather than deriving 100% from the absolute theoretical map capacity.

Yes, its 100% at (exactly) 20 resources, but why 20 and not 10, 30 or 40 when 49 is the theoretical maximum? Or even as low as 7 which would be 1 node with full resources.
I'm not decided on what makes sense, but it needs to be explained in the code where the constant is defined with a short sentence.
And as the initial version was by @Spikeone I'd follow his preference if he has any.

[DevOpsOfChaos]

Updated the PR to address the requested architecture cleanup and the S4-like chance clarity point.

Main changes:

• removed the separate GRANITEMINES_WORK_EVERYWHERE addon path
• reused INEXHAUSTIBLE_GRANITEMINES as the granite mine behavior list ID
• old boolean value 1 now maps to Inexhaustible
• removed active registration/use of global INEXHAUSTIBLE_MINES
• legacy enabled global INEXHAUSTIBLE_MINES settings are migrated to per-mine Inexhaustible selections when loading/deserializing settings
• replaced the duplicated quiet resource lookup with FindPointWithResource(..., false)
• simplified mine behavior helper logic
• corrected Work everywhere semantics so it skips resource checks instead of creating/depleting world resources
• kept S4-like behavior and no-output fallback behavior
• made the S4-like chance reference explicit via a named 20-resource full-productivity reference instead of a buried “5% per resource” rule
• updated Production, AI, Serialization, and UI/productivity tests

S4-like behavior after the rework:

• each mine type is configured independently
• production chance is based on remaining matching resources in the mining radius
• 20 remaining matching resources is the explicit full-productivity reference
• successful production depletes resources down to 1, not 0
• failed production runs the configured no-output/fallback cycle

Validation:

• clang-format version 10.0.0
• built Test_integration
• built Test_UI
• Test_integration --run_test=Production passed
• Test_integration --run_test=AI passed
• Test_integration --run_test=Serialization passed
• ctest -R "^Test_integration$" passed
• Test_UI.exe --report_level=short passed
• git diff --check upstream/master...HEAD clean

[Flamefire]

Most things are now readability:

• TODO(Replay) and/or TODO(Savegame) allows to quickly find all places
• "display"productivity seems odd
• less code is usually better: Reduce C&P and avoid enum constraints where not necessary or document why
• The new notify param for FindPointWithResource now looks redundant so notification might get moved out which is better for SRP as a "find" should not "notify"

[DevOpsOfChaos]

Updated the branch against current master and addressed the remaining mine-resource review points.

The follow-up now uses one productivity path for the expected primary-resource yield, removes the parallel display-productivity layer, and keeps fallback goods outside that primary-resource metric.

It also fixes the S4-like resource calculation to consider all matching resource points within the mine radius, with regression coverage for multiple points, wrong resource types, out-of-range points, empty resources, and rounding.

Validated locally:

• Production suite: 16 test cases / 179 assertions
• AI suite
• targeted mine-productivity and legacy serialization scenarios
• Test_integration via ctest
• Test_UI build
• git diff --check: clean

[DevOpsOfChaos]

Followed up on the remaining review-level cleanup points.

• The legacy INEXHAUSTIBLE_MINES compatibility marker now explicitly covers both replay and savegame migration.
• Removed the public getter that only exposed the internal S4-like full-productivity reference constant; the named constant is now local to the implementation.
• Kept FindPointWithResource(..., notify) unchanged because the former quiet duplicate is already removed and the current callers intentionally use the shared function.

Validated locally:

• Production: 16 test cases / 179 assertions
• Serialization: 10 test cases / 15917 assertions
• AI: 11 test cases / 21376 assertions
• Test_integration ctest: passed
• Test_UI: 73 test cases / 11106 assertions
• git diff --check upstream/master...HEAD: clean

[Flamefire]

There are a couple minor points left that need to be addressed so we can get this merged. Just in case they got missed

[DevOpsOfChaos]

Final verification update for head 9305cea8a.

In addition to addressing the remaining inline review points, the branch was tested through a temporary local integration with current master (4b314647). That exposed five new boost::none returns which no longer compiled after master migrated helpers::OptionalEnum to std::optional; they now use {}, compiling both standalone and with current master.

Lua backward compatibility was also preserved. ADDON_INEXHAUSTIBLE_GRANITEMINES remains available as an alias for ADDON_GRANITEMINE_RESOURCE_BEHAVIOR, and rttr:SetAddon(ADDON_INEXHAUSTIBLE_MINES, true) now uses the same legacy migration as settings/savegames without re-registering the obsolete addon. A Lua regression test covers the alias, all four mine types, explicit-setting preservation, the false case and the unregistered legacy addon.

Locally verified with GCC 14 and Clang 18, Debug and warnings-as-errors: complete build, ctest 20/20, Production, AI, Serialization and Lua settings suites. Earlier verification additionally covered Release, ASan/UBSan, clang-tidy and Test_UI. Jenkins and MSVC were not available locally, so this comment makes no claim about them.

[Flamefire]

Ok, now mostly some changes to make the test more self-documenting and clean. A few things look odd there.

Also LuaInterfaceGameBase::GetFeatureLevel must be increased to cater for the new constants and this change should be documented too

[DevOpsOfChaos]

Addressed all review points:

• GetFeatureLevel bumped 6 → 7 with documenting comment
• nofMiner::ProduceWare extracted shared — test and production now reuse it
• testAI.cpp — renamed , added motivation comments, combined granite tests, replaced magic with
• testGamePlayer.cpp — dropped dead BQ search, introduced with / lambdas, added constant
• testProduction.cpp — merged fixtures into , named constants replace magic numbers, fallback tests collapsed into , exhaustion tests paired with shared constants
• testSerialization.cpp — rewrote misleading magic-number comment to reference actual symbol

Locally verified with GCC 14 and Clang 18 as before — ctest run pending submodule init. Ready for re-review.

[Flamefire]

Sorry for the delay, vacation time and such...
I went over the changes and resolved adressed comments. A few are still open or new, but should be quick to resolve.
And it needs a clang-format run, see CI

[DevOpsOfChaos]

No worries about the delay, hope the vacation was good.

All the open points are in, plus the clang-format run.

On the AI tests: combining them does work, it just needs a fresh AI per setting. AIPlayerJH rates the nodes once on construction, so flipping the addon afterwards had no effect and the assertion was silently testing nothing — that was the reason for the split. It is one test again now, with a small lambda that builds its own AI per behavior, so the WorkEverywhere/Inexhaustible difference sits side by side.

One thing I left alone: the map size in MineProductivityFixture. I inlined the alias and shortened the comment, but kept the literal 20x12 — with MINER_RADIUS == 2 anything expressed in terms of it either gets too small for the HQ offset the test relies on, or needs fairly arbitrary multipliers. Happy to change it if you had a particular form in mind.

One caveat: I could not build locally this time (the machine I am on has neither MSVC nor Boost set up), so the test run is on CI here.

[DevOpsOfChaos]

Small correction to my previous comment: I did get a local build working after all, so the caveat at the end of it is outdated.

Built and ran on Linux with GCC 15.2 and Boost 1.90:

• ctest: 20/20 passed (incl. Test_UI, Test_lua, Test_drivers)
• Test_integration on its own: 215/215 test cases, 93537 assertions
• Production 17/17, AI 10/10, Serialization 11/11

The combined AI test and the reworked out-of-range check in testGamePlayer both pass — so combining the two AI tests really does work as long as each behavior gets its own AI instance.

One thing you may want to know: the Actions runs for b066a8e are currently sitting in action_required, so they need your approval before CI picks the commit up.

[Flamefire]

On the AI tests: combining them does work, it just needs a fresh AI per setting. AIPlayerJH rates the nodes once on construction, so flipping the addon afterwards had no effect and the assertion was silently testing nothing — that was the reason for the split. It is one test again now, with a small lambda that builds its own AI per behavior, so the WorkEverywhere/Inexhaustible difference sits side by side.

Great! Makes sense of course but isn't immediately clear. Can you put a comment with that finding there? E.g. AI assumes game settings do not change, so need to create it (a new instance) after the settings are changed

I left a previous comment there on also testing Inexhaustible. That is currently incomplete: It only tests that there are no places when inexhaustible is set on NO resources, but not that there are places if there are resources. That's a trivial extension to the test. So can you add that? Unless I missed that this is already tested somewhere else. But it surely does make sense to add the 3-4 lines there to highlight the differences.

One thing I left alone: the map size in MineProductivityFixture. I inlined the alias and shortened the comment, but kept the literal 20x12 — with MINER_RADIUS == 2 anything expressed in terms of it either gets too small for the HQ offset the test relies on, or needs fairly arbitrary multipliers. Happy to change it if you had a particular form in mind.

I would have imagined something like MINER_RADIUS * 2 + x. Otherwise you'd need an assertion that size.x/y > MINER_RADIUS * 2 (if that's what you meant) to ensure your assumption does not break.
It basically comes down to non-local assumptions: Someone changing MINER_RADIUS might not be aware of tests relying on it. See also the new comment below that.

One thing you may want to know: the Actions runs for b066a8e are currently sitting in action_required, so they need your approval before CI picks the commit up.

@Flow86 I think it makes sense to disable that requirement until we see the need for that by e.g. a flood of trash PRs. Currently it seems more like a nuisance.

[Flamefire]

For reference with the above asserts maybe:

Suggested change

	BOOST_TEST(!findsMineSpotFor(MineResourceBehavior::Inexhaustible, AIResource::Granite));
	BOOST_TEST(!findsMineSpotFor(MineResourceBehavior::Inexhaustible, AIResource::Granite));
	BOOST_TEST(!findsMineSpotFor(MineResourceBehavior::Inexhaustible, AIResource::Coal));

And the opposite is missing: Have a single split with granite. Or is there another AI test for Inexhaustible granite with some granite somewhere?

If not I'd suggest to place granite somewhere , e.g. neighbor to searchCenter and test:

    BOOST_TEST(findsMineSpotFor(MineResourceBehavior::Inexhaustible, AIResource::Granite));
    BOOST_TEST(!findsMineSpotFor(MineResourceBehavior::WorkEverywhere, AIResource::Coal));

[Flamefire]

Why not use MINER_RADIUS here? This assumption (Is it 4 > MINER_RADIUS?) may break in the future
And I don't understand why this is a requirement at all: What does the HQ has to do with this and why wouldn't a node be able to hold coal? I.e. what is the difference between the nodes at hq + 4 and hq + 3?