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.

Sign up for GitHub

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

Jump to bottom

[Bug]: High level pathfinder passing patches with no water as the origin. #11862

Open
SamuXarick opened this issue Jan 21, 2024 · 1 comment · May be fixed by #12156
Open

[Bug]: High level pathfinder passing patches with no water as the origin. #11862

SamuXarick opened this issue Jan 21, 2024 · 1 comment · May be fixed by #12156

Comments

@SamuXarick
Copy link
Contributor

Version of OpenTTD

20240121-master-g0841978304

Expected result

High level pathfinder always passes patches with water.

Actual result

High level pathfinder passes patches with no water as the origin. When getting neighbours with a patch like that, it eithers gets those where both regions have 1 patch, or none.

Steps to reproduce

OpenTTD/src/pathfinder/yapf/yapf_ship_regions.cpp

Lines 212 to 213 in c7d5ced

TileIndex tile = v->dest_tile;
pf.AddOrigin(GetWaterRegionPatchInfo(tile));

Load savegame, when the ship departs from the dock, it creates a scenario where the order type is changed to OT_LEAVING, but v->dest_tile points to the slope of the dock, which has no water.
ships crashing water regions.zip
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Jan 21, 2024
@SamuXarick
Fix OpenTTD#11862: Don't pass patches without water as the origin
db7031e 
The water region pathfinder expects the origin tile to have a patch of water, but under certain circumstances, this may not happen. This would make the gathering of neighbouring regions incorrect.

When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVING OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

Try to look at the order destination unconditionally of order type and determine whether to check for docking tiles of the tile pointed by v->dest_tile, add the v->dest_tile or add no tile at all as the origin node.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Jan 31, 2024
@SamuXarick
Fix OpenTTD#11862: Don't pass patches without water as the origin
2522a1f 
The water region pathfinder expects the origin tile to have a patch of water, but under certain circumstances, this may not happen. This would make the gathering of neighbouring regions incorrect.

When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVING OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

Try to look at the order destination unconditionally of order type and determine whether to check for docking tiles of the tile pointed by v->dest_tile, add the v->dest_tile or add no tile at all as the origin node.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Jan 31, 2024
@SamuXarick
Fix OpenTTD#11862: Don't pass patches without water as the origin
2c1cb78 
The water region pathfinder expects the origin tile to have a patch of water, but under certain circumstances, this may not happen. This would make the gathering of neighbouring regions incorrect.

When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVING OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

Try to look at the order destination unconditionally of order type and determine whether to check for docking tiles of the tile pointed by v->dest_tile, add the v->dest_tile or add no tile at all as the origin node.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Jan 31, 2024
@SamuXarick
Fix OpenTTD#11862: Don't pass patches without water as the origin
ee04a4f 
The water region pathfinder expects the origin tile to have a patch of water, but under certain circumstances, this may not happen. This would make the gathering of neighbouring regions incorrect.

When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVING OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

Try to look at the order destination unconditionally of order type and determine whether to check for docking tiles of the tile pointed by v->dest_tile, add the v->dest_tile or add no tile at all as the origin node.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Jan 31, 2024
@SamuXarick
Fix OpenTTD#11862: Don't pass patches without water as the origin
255b398 
The water region pathfinder expects the origin tile to have a patch of water, but under certain circumstances, this may not happen. This would make the gathering of neighbouring regions incorrect.

When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVING OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

Try to look at the order destination unconditionally of order type and determine whether to check for docking tiles of the tile pointed by v->dest_tile, add the v->dest_tile or add no tile at all as the origin node.
@Kuhnovic
Copy link
Contributor

Kuhnovic commented Feb 15, 2024 •
edited

I had a look at this, the problem is much weirder than I thought. Turns out the problem is not related to the new pathfinder at all. I put back the old ship pathfinder implementation (easy to do, it's only yapf_ship.cpp... you only need to do a couple of minor fixes). Note that the old pathfinder's SetDestination function is very similar to what the high level pathfinder is doing, and where @SamuXarick is trying different things.
image
This is the savegame provided in the description. I had to lower some land to make sure the pathfinder's node limit wouldn't be the problem. And sure enough, the ship still gets lost! I suspect it to be some edge case that has to do with the conditional orders and the fact that the ship is empty.

I did build a frankenstein build so it's not 100% sure that this problem exists in older openttd version, but it is highly likely.

@Kuhnovic Kuhnovic mentioned this issue Feb 17, 2024
Merged
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Feb 22, 2024
@SamuXarick
Fix OpenTTD#11862: Don't pass patches without water as the origin
b26f285 
The water region pathfinder expects the origin tile to have a patch of water, but under certain circumstances, this may not happen.

When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVESTATION OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Feb 22, 2024
@SamuXarick
Fix OpenTTD#11862: Ships leaving docks could be momentously lost
60fedee 
When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVESTATION OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

OT_LEAVESTATION OrderType still has the same destination as that of OT_GOTO_STATION, so assume they still belong to the same dock.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Feb 22, 2024
@SamuXarick
Fix OpenTTD#11862: Ships leaving docks could be momentarily lost
a5a016f 
When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVESTATION OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

OT_LEAVESTATION OrderType still has the same destination as that of OT_GOTO_STATION, so assume they still belong to the same dock.
@SamuXarick SamuXarick linked a pull request Feb 23, 2024 that will close this issue
Open
@Kuhnovic Kuhnovic mentioned this issue Mar 8, 2024
Merged
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Mar 8, 2024
@SamuXarick
Fix OpenTTD#11862: Ships leaving docks could be momentarily lost
eeb4ccf 
When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVESTATION OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

OT_LEAVESTATION OrderType still has the same destination as that of OT_GOTO_STATION, so assume they still belong to the same dock.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this issue Mar 9, 2024
@SamuXarick
Fix OpenTTD#11862: Ships leaving docks could be momentarily lost
9eb9c03 
When a ship is departing from a dock, and wants to go back to it again, the destination tile is set as the origin, making the ship have a OT_LEAVESTATION OrderType. With this order type, the docking tiles aren't added as origins even though the tile is still that of a dock.

OT_LEAVESTATION OrderType still has the same destination as that of OT_GOTO_STATION, so assume they still belong to the same dock.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #11862: Ships leaving docks could be momentarily lost SamuXarick/OpenTTD
2 participants
@SamuXarick @Kuhnovic