Skip to content

Adding module for allowing searoute connection for towns#136

Merged
YourDeveloperFriend merged 7 commits intoYourDeveloperFriend:mainfrom
Grimiku:new_module
Aug 15, 2025
Merged

Adding module for allowing searoute connection for towns#136
YourDeveloperFriend merged 7 commits intoYourDeveloperFriend:mainfrom
Grimiku:new_module

Conversation

@Grimiku
Copy link
Contributor

@Grimiku Grimiku commented Jul 22, 2025

Hi @YourDeveloperFriend, I think this is what you meant in #131 (comment)?

Although I see only Portugal and Denmark utilized it before. Can't see anything like it on HeavyCardboard but it used to be the case in Scotland, although I think you re-implemented most of the logic on that map due to some feedback.

If anything else is needed or you remember some other maps that won't be needing validateUrbanizedCities(), let me know - I can add this to the PR.

@YourDeveloperFriend
Copy link
Owner

I think what's missing is that the logic for building track towards the ocean, and the logic for moving goods across that path, are still contained within the individual maps. Ideally, that logic can be centralized also.

@Grimiku
Copy link
Contributor Author

Grimiku commented Aug 6, 2025

Sure, will take a look next week, I'm on vacay without laptop unfortunately.

@Grimiku
Copy link
Contributor Author

Grimiku commented Aug 13, 2025

Ok, so the basics seem to be implemented.

Couple of notes:

a) so far modified only Portugal to use this module for testing purposes and it reflects current behavior.

b) I didn't find a good way to check for exit towards InterCityConnection so I had to modify everything to do with InterCityConnection to reflect a new optional property called connectedTownExit. When defining those connections on any new map the person implementing would have to specify a Direction (or an array of Directions) where those exits actually exit for a Town. So, for example, in Portugal as both Sines and Sagres have BOTTOM towards the sea route this is now added as such. Currently for Denmark it is resolved with a map specific property which basically does the same thing and for Portugal it was somewhat hardcoded in PortugalBuildValidator.

c) Because of point b) something will have to be done about all current games as the new property is not reflected in data for them ;(

d) There is this interesting approach in current Denmark implementation where the link from a Town towards a SeaRoute is automatically "unowned". As the player you pay for that part but you don't get a point for it etc. Is this expected @JackOfMostTrades? Should we also somehow add this to the module?

In general, I'd like to check with you guys @YourDeveloperFriend and @JackOfMostTrades what you make of it and should we modify Denmark and HeavyCardboard to have the module.

@Grimiku Grimiku changed the title Adding module for omitting validation for cities during ConnectCitiesAction Adding module for allowing searoute connection for towns Aug 13, 2025
@JackOfMostTrades
Copy link
Collaborator

There is this interesting approach in current Denmark implementation where the link from a Town towards a SeaRoute is automatically "unowned". As the player you pay for that part but you don't get a point for it etc. Is this expected @JackOfMostTrades? Should we also somehow add this to the module?

Yea, that's a particular quirk and rule of the Denmark map. From Denmark's official rules:

A town is connected to the adjacent end of a sea route if there is track connecting the town to the end of the
sea route. ... Track markers are not placed on the stubs connecting a town to a sea route.

So in the Denmark map, the town-to-sea-link stubs are required to have been built to be able to use a sea link, but don't get ownership markers. On Portugal, the current implementation makes it look like the stubs aren't required to be able to use a sea link, and can be built by any player who then retains ownership for the purposes of end-game points, but the ownership otherwise doesn't matter for income during deliveries. Is that the intended behavior on Portugal?

@Grimiku
Copy link
Contributor Author

Grimiku commented Aug 14, 2025

Correct! For delivery only the ownedintercityconnection part is returned to routes so the regular part from stub to the searoute is not taken into consideration. But I noticed I missed adding danglers overwrite which doesn't have to be done on Denmark due to unowned-by-default logic. Will add today hopefully.

@YourDeveloperFriend
Copy link
Owner

Overall this looks great. I didn't look closely enough to see if it works as intended, so please be sure to test the behavior locally.

@Grimiku
Copy link
Contributor Author

Grimiku commented Aug 15, 2025

Ok, so I managed to add danglers omission and implemented the module in Portugal + tested if it works same as the current implementation. I think we can potentially merge now if you're ok with this @YourDeveloperFriend as we don't seem to have any ongoing Portugal games (at least listed).

Implementing for Denmark would require some removals here and there (like the map specific property etc.). Would be good to have your confirmation @JackOfMostTrades if you're ok with adding the module on that map and aligning it.

HeavyCardboard can be done quickly I think?

@YourDeveloperFriend YourDeveloperFriend merged commit 6cf970f into YourDeveloperFriend:main Aug 15, 2025
1 of 2 checks passed
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.

3 participants