Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

joining road/ship by placing settlement doesnt recalculate longest route #95

Closed
kotc opened this issue Apr 19, 2021 · 7 comments
Closed
Assignees
Labels
bug pending-release Fix is tested and will be part of the next JSettlers release.

Comments

@kotc
Copy link
Contributor

kotc commented Apr 19, 2021

happened during the game, got caught because longest route was going to win the game.

should be easy to replicate:

1/ make player1 have longest route
2/ player2 should build 2 shorter routes that touch in sea/land location
3/ player2 builds settlement, resulting route should be recalculated, but isnt so
4/ placing any road/ship will recalculate longest route

most likely longest route should be recalculated also at placing settlements, not only roads/ships

jdmonin added a commit that referenced this issue Apr 27, 2021
…te when placing coastal settlement to connect a road with ships: Add savegame artifact reletest-longest-joinships to repro
@jdmonin jdmonin self-assigned this Apr 27, 2021
@jdmonin jdmonin added the bug label Apr 27, 2021
@jdmonin
Copy link
Owner

jdmonin commented Apr 27, 2021

Thanks for noticing this. The Release-Testing checklist covers most longest-route changes, but missed this case.

I'm looking into it; created src/test/resources/resources/savegame/reletest-longest-joinships.game.json for a quick way to reproduce the bug.

@jdmonin
Copy link
Owner

jdmonin commented Apr 27, 2021

Also, this was probably frustrating to encounter during your game. My apologies for that

@kotc
Copy link
Contributor Author

kotc commented Apr 27, 2021

you shouldnt be apologizing. every software has bugs, but software which has active support is the best, so thank you for keeping it alive :)

@jdmonin
Copy link
Owner

jdmonin commented Apr 27, 2021

Thanks, I really enjoy working on it. Thanks for all your reports and suggestions to make it even better :)

@kotc
Copy link
Contributor Author

kotc commented Apr 27, 2021

yeah, i play it a lot too. few times daily, gf is hooked ;)

jdmonin added a commit that referenced this issue May 2, 2021
… longest route when building a new coastal settlement to connect their roads to ships; add to release-testing; savegame artifact reletest-longest-joinships + author, comment, unlock p2 seat to allow takeover
jdmonin added a commit that referenced this issue May 2, 2021
… longest route when building a new coastal settlement to connect their roads to ships; add to release-testing; savegame artifact reletest-longest-joinships + author, comment, unlock p2 seat to allow takeover
@jdmonin
Copy link
Owner

jdmonin commented May 2, 2021

Thanks again for finding this! Tested the fix, tested regression for other longest-route conditions, added to release-testing.

This will be part of the next released version. Code is server-side, and will work with any client version which supports sea boards.

@jdmonin jdmonin added the pending-release Fix is tested and will be part of the next JSettlers release. label May 2, 2021
@jdmonin
Copy link
Owner

jdmonin commented Dec 31, 2021

This fix was released as part of v2.5.00. Thanks again!

@jdmonin jdmonin closed this as completed Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pending-release Fix is tested and will be part of the next JSettlers release.
Projects
None yet
Development

No branches or pull requests

2 participants