Fix sea path finding (for ships)#1917
Open
Flamefire wants to merge 12 commits into
Open
Conversation
Flamefire
force-pushed
the
fixShipConnections
branch
2 times, most recently
from
4149525 to
3198d59
Compare
Flamefire
force-pushed
the
fixShipConnections
branch
from
3198d59 to
3d707b9
Compare
Not required (anymore), trade path cache can be created on demand.
It could be possible that 2 harbors are at the same 2 seas. The current harbor connections do not honor this and only store a single distance which would hence be wrong for one of those 2. Store the seaId for each connection.
Do not try to limit pathfinding for ships. Instead store the most common routes: Those between harbors in a small, fast lookup table. For other routes, e.g. from shipyards or lost ships, use a LRU cache and fall back to unlimited search.
There is no meaningful result for all out-parameters in the base pathfinding implementation.
When we are already at the goal `WanderOnWater` changes the state to reflect it. So we must ensure to change the state only before, not after the call to this function.
Flamefire
force-pushed
the
fixShipConnections
branch
from
99d95e5 to
bbb8cab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If a harbor is too far we will not find the path even though it exists, see the previous TODO.
Issue happened in the map where there was a triangle of water with land in the middle.
A ship was moving between 2 far away harbors when one was destroyed.
When going home we limit the maximum distance by the longest connections of HomeHarbor which might not be enough
Pathfinding considered the current harbor when looking for connections which is not useful and required extra steps.
--> Removed together with
GamePlayer::AddHarborsAtSeawhich was only used there and added testI found this while debugging the previous seafaring issue.
When letting it continue to run on the triangle-map mostly land with a triangle shaped straight of water it would run into the assert.
Additional:
CreateTradeGraphssimply created a cache instance but must be manually invoked or it will crash at runtime.Remove the method and create the instance on first access for consistency