HPF handles searches from unreachable source cells into cut off areas.#21495
Merged
Conversation
The scenario is that an actor is on an unreachable tile, and would like to path. As long as it is immediately adjacent to some reachable tiles, it can still move onto them and path. Now imagine a map split into two by a one tile wide line of impassable cliffs. It is important which side it chooses to jump into, as once it has moved off the cliff it loses access to the other side. Jumping off the correct side will allow a valid path, jumping off the wrong side will prevent a path from being possible. In d8ebb96, handling was added to prevent a crash where the path search would simulate having the actor jump off the wrong side and then get confused that it could not find a path when one was expected. This fix works by remembering the `unpathableNodes` - the nodes where you jump onto the wrong side. If we encounter them later, we can ignore them. In 5157bc3 we added domain checks - this allows the HPF to bail on impossible paths early by checking if they belong to different domains. For example islands on a water map will belong to different domains. This caused a regression where `sourcesWithReachableNodes` was now badly named. Some reachable nodes because they were in the wrong domain. Later when `sourcesWithPathableNodes` and `unpathableNodes` are built - we don't populate the `unpathableNodes` correctly, because we already excluded the unpathable nodes earlier on! This means we don't ignore them any more, and we reintroduce the crash. Now that we are checking the domain, we can simplify the code and resolve the crash at the same time. `sourcesWithReachableNodes` becomes `sourcesWithPathableNodes` because the domain check has been done. We can build `unpathableNodes` at the same time. This allows us to remove the later code that explored the abstract graph, as the domain check succinctly achieves the same end goal of determining whether the node has a path or not.
pchote
approved these changes
Jul 25, 2024
pchote
left a comment
pchote
left a comment
Member
There was a problem hiding this comment.
Code changes paired with the detailed PR description make sense, but not tested ingame.
Member
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
3 participants
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.
The scenario is that an actor is on an unreachable tile, and would like to path. As long as it is immediately adjacent to some reachable tiles, it can still move onto them and path. Now imagine a map split into two by a one tile wide line of impassable cliffs. It is important which side it chooses to jump into, as once it has moved off the cliff it loses access to the other side. Jumping off the correct side will allow a valid path, jumping off the wrong side will prevent a path from being possible.
In #20498, handling was added to prevent a crash where the path search would simulate having the actor jump off the wrong side and then get confused that it could not find a path when one was expected. This fix works by remembering the
unpathableNodes- the nodes where you jump onto the wrong side. If we encounter them later, we can ignore them.In #21164 we added domain checks - this allows the HPF to bail on impossible paths early by checking if they belong to different domains. For example islands on a water map will belong to different domains.
This caused a regression where
sourcesWithReachableNodeswas now badly named. Some reachable nodes were excluded because they were in the wrong domain. Later whensourcesWithPathableNodesandunpathableNodesare built - we don't populate theunpathableNodescorrectly, because we already excluded the unpathable nodes earlier on! This means we don't ignore them any more, and we reintroduce the crash.Now that we are checking the domain, we can simplify the code and resolve the crash at the same time.
sourcesWithReachableNodesbecomessourcesWithPathableNodesbecause the domain check has been done. We can buildunpathableNodesat the same time. This allows us to remove the later code that explored the abstract graph, as the domain check succinctly achieves the same end goal of determining whether the node has a path or not.Fixes #21493 - running the replay with this fix applied no longer crashes and it ends cleanly. It may also be the solve for #21459. It depends whether the bug was this one, or due to the modified engine, I haven't investigated.