Add optional free harbor spots addon

[DevOpsOfChaos]

Summary

• add an optional FREE_HARBOR_SPOTS addon
• generate real harbor points for suitable coastal castle sites when no map-defined harbor marker exists
• keep the default harbor-marker behavior unchanged unless the addon is enabled
• explicitly label the addon as dangerous/advanced because it can heavily alter intended map seafaring design
• add regression coverage for generated harbor spots and markerless island maps

Motivation

Some maps contain coastal locations that are physically suitable for harbor buildings, but do not define explicit harbor markers. On such maps, players may be unable to build a harbor even though the terrain would otherwise support one.

This addon provides an optional fallback for that case: when enabled during harbor initialization, suitable coastal castle sites can be added as real harbor points.

This can significantly change how a map plays, especially for seafaring maps. For that reason, the addon is disabled by default and its in-game name/description now explicitly warns that it is a dangerous advanced option.

Scope / safety notes

This PR intentionally keeps the behavior limited:

• default behavior is unchanged when the addon is disabled
• existing map-defined harbor spots remain the preferred/normal behavior
• the addon is explicitly labelled as dangerous/advanced because it may heavily alter intended map seafaring design
• the addon does not make arbitrary coast tiles buildable
• the site still has to satisfy the normal suitability checks for a coastal castle/harbor location
• generated spots are inserted into harborData during sea/harbor initialization
• generated spots receive normal harborId, seaId, coastal-point and neighbor data through the existing harbor initialization path
• runtime BQ recalculation does not invent additional harbor spots
• this does not change ship movement, expedition logic, harbor building logic, or maps

The intent is to support markerless-but-suitable coastal maps without weakening the normal harbor placement rules globally.

A separate generic addon-warning popup could be explored later, but this PR only adds the explicit warning text to avoid expanding the scope into addon/UI behavior.

Validation

• Built Test_integration locally with Visual Studio 2022 / Debug
• Ran Test_integration.exe --run_test=SeaWorldCreationSuite --report_level=short
• Result: 5 test cases passed, 1521 assertions passed
• Ran clang-format 10.0.0
• Ran git diff --check

[DevOpsOfChaos]

Thanks, addressed the review points.

• clarified the addon description with “all suitable coastal castle sites”
• renamed the BQCalculator option to allowHarborsWithoutMapMarkers
• simplified the harbor BQ branch without changing behavior
• replaced the heavier SeaWorldWithGCExecution<> fixture for the new tests with lighter world fixtures

Semantics are unchanged: generated harbor spots are still created only during sea/harbor initialization, and runtime BQ recalculation does not invent additional harbor spots.

Validation:

• Built Test_integration locally with Visual Studio 2022 / Debug
• Ran Test_integration.exe --run_test=SeaWorldCreationSuite --report_level=short
• Result: 5 test cases passed, 1521 assertions passed

[Spikeone]

Isn't there quite a potential for breaking the game? Not sure if we correctly check if harbors are very close or not, but I think this is a dangerous option for people who do not know how the seafaring and harbors actually work.

Although I do like the idea, I'd still not use such addon and rather go for a modified version of a map.

[DevOpsOfChaos]

Isn't there quite a potential for breaking the game? Not sure if we correctly check if harbors are very close or not, but I think this is a dangerous option for people who do not know how the seafaring and harbors actually work.

Although I do like the idea, I'd still not use such addon and rather go for a modified version of a map.

That is a fair concern.

The addon is disabled by default and it only adds spots during harbor initialization, but I agree that this can still change map design quite heavily for players who do not understand the seafaring rules.

I can either make the addon text more explicit that this is an advanced/map-altering option, or close this PR if the preferred direction is to solve these cases with modified maps instead.

[Spikeone]

Best would be showing a hint when enabling it I think. But we don't have that yet, not sure how hard it is to implement this. Also people sadly do not read the description in most cases.

If we can have a popup, I'd go for that.

If not, we maybe can prefix it with "Dangerous: ..."? Maybe this makes people question what this is about and they read the hint,

[DevOpsOfChaos]

Best would be showing a hint when enabling it I think. But we don't have that yet, not sure how hard it is to implement this. Also people sadly do not read the description in most cases.

If we can have a popup, I'd go for that.

If not, we maybe can prefix it with "Dangerous: ..."? Maybe this makes people question what this is about and they read the hint,

Done. I prefixed the addon with Dangerous: and made the description more explicit about the map/seafaring risk.

I’ll keep the popup idea separate from this PR. A small generic addon-warning popup could be worth exploring as a follow-up, then we can review whether that direction makes sense before tying it to this addon.

[Flamefire]

Isn't there quite a potential for breaking the game? Not sure if we correctly check if harbors are very close or not, but I think this is a dangerous option for people who do not know how the seafaring and harbors actually work.

Very valid concern I'd say. E.g. exploration expeditions go to the "next" unexplored harbor spot, and (founders) expedition are steered towards the next (free) harbor spot. If now suddenly there are so many of them this could become unusable.

I'm not sure if or for which maps this addon would be useful. Any example?
I'd rather not add things "just because"

[DevOpsOfChaos]

Isn't there quite a potential for breaking the game? Not sure if we correctly check if harbors are very close or not, but I think this is a dangerous option for people who do not know how the seafaring and harbors actually work.

Very valid concern I'd say. E.g. exploration expeditions go to the "next" unexplored harbor spot, and (founders) expedition are steered towards the next (free) harbor spot. If now suddenly there are so many of them this could become unusable.

I'm not sure if or for which maps this addon would be useful. Any example? I'd rather not add things "just because"

That is a fair concern, and I agree we should not add this only because it is technically possible.

I ran a local audit with Codex to get concrete data instead of guessing. The temporary audit loaded the available .swd/.wld maps through the real MapLoader path, once with FREE_HARBOR_SPOTS disabled and once with it enabled, then compared the harbor counts. I removed the temporary audit source afterwards and kept only the generated report for review.

The scan found real examples where maps have no map-defined harbor spots but would get generated candidates:

• DuskTillDawn.SWD: 0 existing harbors, 10 generated with the addon, 8 significant land components
• Insel2.swd: 0 existing harbors, 22 generated with the addon, 2 significant land components
• INSEL6-6.SWD: 0 existing harbors, 39 generated with the addon, 3 significant land components
• INSEL5-2.SWD: 0 existing harbors, 57 generated with the addon, 4 significant land components
• KARIBIK2.SWD: 0 existing harbors, 66 generated with the addon, 9 significant land components

So there are existing island/coastal maps where this addon has a concrete use case.

However, the audit also confirms the risk: some maps would receive a very large number of generated harbor spots, which could indeed make expedition behavior worse or change the map too much.

Maybe the better direction is not “allow every suitable coastal castle site”, but to add stricter generation rules. For example, generate only a small number of harbor spots per land component, enforce a minimum distance between generated harbors, and maybe limit small islands to one harbor at most while larger landmasses get only a few well-spaced candidates.

I attached the audit report here so we can discuss whether the current broad version is acceptable, whether it should be constrained further, or whether this should stay out of upstream.

free_harbor_spots_map_audit.md

[Spikeone]

So there are existing island/coastal maps where this addon has a concrete use case.

Those are very few maps aren't they? Also those "Insel" Maps are rarely played. Thinking about this, I now also fear that people to expect this to work on most maps (e.g. they may think they can make "The green island" a seafaring map) but it fails most of the time. 5 / 444 maps are only some rare edge cases I think.

If those maps are truly meant for seafaring, we should modify them I think.

[Flamefire]

I feel similar: If there is a seafaring map then harbors will be placed by the author. Very likely in places where it makes sense and on purpose.
"Spamming" the map with harbor spots doesn't seem sensible.

Was something like this ever requested? If so in which situation?

[DevOpsOfChaos]

I feel similar: If there is a seafaring map then harbors will be placed by the author. Very likely in places where it makes sense and on purpose. "Spamming" the map with harbor spots doesn't seem sensible.

Was something like this ever requested? If so in which situatio

I feel similar: If there is a seafaring map then harbors will be placed by the author. Very likely in places where it makes sense and on purpose. "Spamming" the map with harbor spots doesn't seem sensible.

Was something like this ever requested? If so in which situation?

I’ve noticed this on a few maps myself, so the idea came from my own observation rather than a specific request. In some of those cases, I think additional harbor spots could have been useful.

That said, I agree with your overall point: on a seafaring map, harbor placement is usually intentional and adding more indiscriminately would likely reduce map quality. It also probably wouldn’t have as much positive impact as one might expect.

Given that this only affects a small number of maps, adjusting the system to avoid “harbor spam” doesn’t seem worth the effort.

[Flamefire]

Ok, so it was basically your own request.

Well, if we add this we should at least be sensible, i.e. this makes sense:

Maybe the better direction is not “allow every suitable coastal castle site”, but to add stricter generation rules. For example, generate only a small number of harbor spots per land component, enforce a minimum distance between generated harbors, and maybe limit small islands to one harbor at most while larger landmasses get only a few well-spaced candidates.

However this might be significant effort to identify landmasses/islands. There is already a quite elaborate algorithm for the map generator which works on "coastlines" but that can't be used here directly.

Not sure if this is worth the effort.
But if we want something "cheap" then maybe: Collect all possible new harbor spots (coasts with castle BQ), determine an amount of new harbors (maybe based on map size? sea size? existing number of harbors?) and randomly select some rejecting those that are too close to an existing one. This would however not place ones on different islands opposite a small strip of water but further away. Should be OK though.
The randomness has the advantage that this makes games different for each start, which could be nice.
Or seed the RNG by something map specific to select random harbors but the same/deterministic for each map to avoid having to create some policy for that.

@Spikeone

they may think they can make "The green island" a seafaring map

That wouldn't work? Why?

5 / 444 maps are only some rare edge cases I think.

That's the maps that had no harbors at all. So on others there were harbors, and now are more.

That would also be an option: Limit it to maps with original harbors.

Either way the add-on could then be described as "(add) extra harbors"

Just some thoughts.
What do you 2 think? And maybe @Flow86 has an opinion too?

[Spikeone]

That wouldn't work? Why?

| `C:/Program Files/Spiele/Siedler\DATA\MAPS3\AKARTE03.WLD` | 96x96 | 4 | land components 1 (1 significant), seas 3 |

So this is a map that feels quite coastal (but has no large building spots). I just wanted to say, that players might expect such maps to have seafaring with this addon enabled.

Not sure if this is worth the effort.
But if we want something "cheap" then maybe: Collect all possible new harbor spots (coasts with castle BQ), determine an amount of new harbors (maybe based on map size? sea size? existing number of harbors?) and randomly select some rejecting those that are too close to an existing one. This would however not place ones on different islands opposite a small strip of water but further away. Should be OK though.
The randomness has the advantage that this makes games different for each start, which could be nice.
Or seed the RNG by something map specific to select random harbors but the same/deterministic for each map to avoid having to create some policy for that.

I like this idea to spread them somewhat evenly on a landmass. Also this should have some validation, if a harbor can connect any roads at all - maybe the harbor would block itself or trees are blocking buildings, so you'd have a harbor that can not be used. That even spread is important so you may not have most harbors on one side. But implementing this could be hard in some cases (e.g. if one side of the island is a mountain area and the other is flat for harbors).

Could we preview the amount of generated harbors? Or is it smarted to only generate them after the game actually starts so disabling/enabling the addon does not cause that generation?

As long as things are an addon, I'm fine with any addition - just want to make sure those addons don't confuse people or cause false positive bug reports because results don't meet expectation :)

[Flamefire]

So this is a map that feels quite coastal (but has no large building spots). I just wanted to say, that players might expect such maps to have seafaring with this addon enabled.

So it just doesn't make (much) sense as there is only a single island. But you could still use harbors as means of fast(er?) transport, can't you?

I like this idea to spread them somewhat evenly on a landmass. Also this should have some validation, if a harbor can connect any roads at all - maybe the harbor would block itself or trees are blocking buildings, so you'd have a harbor that can not be used. That even spread is important so you may not have most harbors on one side. But implementing this could be hard in some cases (e.g. if one side of the island is a mountain area and the other is flat for harbors).

That might then be too much already. "spread even on a landmass" requires determining what a "landmass" is first. Possible but not trivial. Hence the idea to use randomness and distance thresholds to get (likely) something close to that.
But if there is a castle building spot, I'd guess that we don't need additional checks for blockage.

Could we preview the amount of generated harbors? Or is it smarted to only generate them after the game actually starts so disabling/enabling the addon does not cause that generation?

Preview where? But yes as addons are part of a game they will be generated when loading the map for the game.

As long as things are an addon, I'm fine with any addition - just want to make sure those addons don't confuse people or cause false positive bug reports because results don't meet expectation :)

True. But we could make that clearer in the description. If it has something like "randomly converts some coastal spots to harbor spots" it would convey that results might not be perfect. Or even have that in the tooltip that not all such harbors might be useful or even usable.

[Spikeone]

So it just doesn't make (much) sense as there is only a single island. But you could still use harbors as means of fast(er?) transport, can't you?

Honestly, on a map like the green island seafaring makes more sense in multiplayer than on some other maps. Simply because it is not necessary, but adds a second front with multiple players (if sea attacks are enabled. So normally you may not attack from the upper left to the lower right - but with seafaring you could.

But if there is a castle building spot, I'd guess that we don't need additional checks for blockage.

I think we should, simpyl because even our random maps sometimes generated such spots and having a harbor you can not attack is pretty annyoing - or if you can not build anything next to it.

Preview where? But yes as addons are part of a game they will be generated when loading the map for the game.

Like a message saying "X - Harbor spots generated on Y islands". So at least players know that something happened.

True. But we could make that clearer in the description. If it has something like "randomly converts some coastal spots to harbor spots" it would convey that results might not be perfect. Or even have that in the tooltip that not all such harbors might be useful or even usable.

👍 maybe I'm to fearful, that people don't read those^^

[Flamefire]

Like a message saying "X - Harbor spots generated on Y islands". So at least players know that something happened.

That could happen when starting the game as a message printed to the log. Or do you want a message box?
The latter requires determining islands which, mentioned above, isn't trivial. Do we really need that?
Depends on the approach we choose on the distribution.
Shall we go with that landmass approach? What would you suggest? Maybe:

Place harbors on all spots but keep distance X to harbors on the same island and distance Y to any other harbor in "lexical" order (L->R, T->B)

👍 maybe I'm to fearful, that people don't read those^^

Not our fault if they don't ;-)

[DevOpsOfChaos]

Thanks for the feedback and suggestions.

I have my final exams this week, so I won’t be able to properly continue the discussion or rework this PR until after that.

My current feeling is that the addon should probably be constrained more than the current broad version. The audit shows real markerless island/coastal map cases where generated harbor spots can help, but it also shows that allowing every suitable coastal castle site can create too many harbor spots on some maps.

So I think the better direction might be stricter generation rules, for example limiting generated harbor spots per landmass and enforcing some spacing, instead of simply accepting every suitable candidate.

I’ll take another look after my exams.

[DevOpsOfChaos]

After rereading the discussion, I agree that the current broad version is probably too aggressive.

The audit shows real candidate maps, but it also shows the main problem: generating every suitable coastal castle site can create far too many harbor spots and may hurt expedition behavior.

If this PR should continue, I would rework it toward a much more limited “extra harbor spots” addon instead of “free harbor spots”: collect candidate coastal castle sites, keep existing map harbors preferred, reject candidates too close to existing/generated harbors, and cap the number of generated spots. I would also keep the selection deterministic, not random per game, to avoid replay/network surprises.

If that direction is still considered too niche for upstream, I’m also fine with closing this PR and leaving such cases to modified maps.