Skip to content

Ignore isolated fish resources for fisheries#1925

Open
DevOpsOfChaos wants to merge 3 commits intoReturn-To-The-Roots:masterfrom
DevOpsOfChaos:sidequest/ignore-isolated-fish-resources
Open

Ignore isolated fish resources for fisheries#1925
DevOpsOfChaos wants to merge 3 commits intoReturn-To-The-Roots:masterfrom
DevOpsOfChaos:sidequest/ignore-isolated-fish-resources

Conversation

@DevOpsOfChaos
Copy link
Copy Markdown

@DevOpsOfChaos DevOpsOfChaos commented May 1, 2026

Summary

  • remove fish resources from non-water and isolated one-node water during world resource setup
  • keep connected/larger water with fish usable for fisheries
  • keep fisher runtime checks simple by relying on normalized resource data
  • add regression coverage for isolated fish water versus connected water

Motivation

Fisheries could still produce fish from an isolated one-node water pond if that node had a fish resource. In the original behavior described in #1171, such isolated water should be exhausted without producing fish.

This change keeps larger/connected water usable, but ignores fish resources on isolated water points.

Validation

  • Built Test_integration locally with Visual Studio 2022 / Debug
  • Ran Test_integration.exe --run_test=BuildingSuite/FisherIgnoresIsolatedFishWater --report_level=short
  • Result: 1 test case passed, 6 assertions passed
  • Ran Test_integration.exe --run_test=BuildingSuite --report_level=short

Fixes #1171

Comment thread tests/s25Main/integration/testBuilding.cpp Outdated
Comment thread tests/s25Main/integration/testBuilding.cpp Outdated
@DevOpsOfChaos
Copy link
Copy Markdown
Author

Updated. I cleaned up the test as requested.

The extra nofFarmhand reference is now only kept where it is needed to call GetPointQuality, and the unnecessary unsigned casts are gone. I also fixed the small formatting/include-order cleanup before pushing.

Validation:

  • Built Test_integration
  • Ran BuildingSuite/FisherIgnoresIsolatedFishWater
  • Ran BuildingSuite

@DevOpsOfChaos DevOpsOfChaos force-pushed the sidequest/ignore-isolated-fish-resources branch from 2f16b65 to 58c0217 Compare May 6, 2026 13:57
@Flamefire
Copy link
Copy Markdown
Member

Thinking about this again: Wouldn't it make more sense to "fix" the map during loading?
I.e. remove fish resources in <=1-water places. This would simplify the runtime check to "is there fish?" which then improves performance.
The terrain cannot change at runtime, can it? so we'd only need to do those checks once.

@Spikeone ?

@DevOpsOfChaos DevOpsOfChaos force-pushed the sidequest/ignore-isolated-fish-resources branch from 58c0217 to 7e4777e Compare May 6, 2026 17:29
@DevOpsOfChaos
Copy link
Copy Markdown
Author

Thinking about this again: Wouldn't it make more sense to "fix" the map during loading? I.e. remove fish resources in <=1-water places. This would simplify the runtime check to "is there fish?" which then improves performance. The terrain cannot change at runtime, can it? so we'd only need to do those checks once.

@Spikeone ?

I reworked this in the direction you suggested.

The isolated/non-water fish cleanup now happens once in GameWorld::SetupResources(), so the fisher runtime path can stay simple and only check for fish resources again.

Connected water with fish is unchanged; isolated one-node water and non-water fish resources are normalized away during world resource setup.

Validation:

  • built Test_integration
  • ran BuildingSuite/FisherIgnoresIsolatedFishWater
  • ran BuildingSuite
  • git diff --check upstream/master...HEAD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fisheries work in 1-node water

2 participants