Ignore isolated fish resources for fisheries

[DevOpsOfChaos]

Summary

• remove fish resources from non-water and isolated one-node water during map resource setup
• keep connected/larger water with fish usable for fisheries
• keep fisher runtime checks simple by relying on normalized resource data
• move resource setup/normalization into MapLoader
• 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 removes fish resources from unusable water during map resource setup.

Implementation details

Resource setup is now handled by MapLoader::SetupResources(GameWorldBase&).

The setup step normalizes resources after map loading:

• converts mine resource types as before
• places/fixes water as before
• removes fish resources from:
  • non-water points
  • isolated one-node water points

The fisher runtime path remains simple and only checks whether fish resources exist.

The fish cleanup iterates row-by-row and uses a row-local shortcut: if the previous point in the same row was kept as fish-on-water, the current water fish point does not need the full neighbor-water scan. This shortcut is not carried across row boundaries.

Validation

• Built Test_integration locally with Visual Studio 2022 / Debug
• Ran Test_integration.exe --run_test=BuildingSuite/FisherIgnoresIsolatedFishWater --report_level=short
  • Result: passed, 11 assertions
• Ran Test_integration.exe --run_test=BuildingSuite --report_level=short
  • Result: passed, 1738 assertions
• Ran git diff --check upstream/master...HEAD

Fixes #1171

[DevOpsOfChaos]

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

[Flamefire]

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]

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

[Flamefire]

Please no force-pushes during a review. If necessary a revert works too. This way I have to check more changes.
Doesn't matter much for this small PR though.

With the added method in GameWorld: I think GameWorld::SetupResources() belongs more to MapLoader. Because that is the only context where it makes sense to call it, doesn't it?
So if I'm not missing anything please move those methods. SetupResources can be a public method, and the others can be private.

[DevOpsOfChaos]

Please no force-pushes during a review. If necessary a revert works too. This way I have to check more changes. Doesn't matter much for this small PR though.

With the added method in GameWorld: I think GameWorld::SetupResources() belongs more to MapLoader. Because that is the only context where it makes sense to call it, doesn't it? So if I'm not missing anything please move those methods. SetupResources can be a public method, and the others can be private.

Sorry about the force-push. I’ll avoid force-pushing this PR from here on and use normal follow-up commits instead.

[DevOpsOfChaos]

I moved the resource setup logic from GameWorld into MapLoader as suggested. MapLoader::SetupResources(GameWorldBase&) now owns the setup step, with the resource-normalization helpers kept private there.

The fish cleanup behavior is unchanged: fish on non-water and isolated one-node water is removed, while connected/larger water keeps fish. The row-local optimization for consecutive fish water is still in place.

Validation:

• built Test_integration
• ran BuildingSuite/FisherIgnoresIsolatedFishWater: 11 assertions passed
• ran BuildingSuite: 1738 assertions passed
• git diff --check upstream/master...HEAD