Make tokens more closely follow their leader #4817
Merged
Conversation
This file contains 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
`ZoneWalker.replaceLastWaypoint()` does not need to return a value, so that has been removed. `ZoneWalker.getPath()` and `SelectionSet.getGridlessPath()` never return `null`, so mark them as such and clean up callers to not check for it. `AbstractPoint` is now sealed with `ZonePoint` and `CellPoint`. We do a lot of casework that already assumes only those two possibilities, and now that is formal. It will also allow for pattern matching.
Avoid some case work and unnecessary `Path<>` creation.
Instead it keeps its own last point that it can freely modify but that is kept out of the path until committed as a waypoint. Gridless path rendering is also cleaned up by using a Path2D. Even better would be to delegate to `ZoneRenderer.renderPath()` like we do for gridded paths, but that current assumes it needs to adjust for a token footprint, which `MeasureTool` does not require.
Also change DTO translation to work with the field rather than the public API.
Just like `MeasureTool`, `SelectionSet` keeps its own current point that it modifies and only adds to the path when set as a waypoint. If the final path is requested, the path is copied and the current point is added as the final waypoint. A subtle aspect of this change is that all points in the `Path` are waypoints and there are no "dangling" cell points.
Now that all users of `Path` do not modify the `Path` and are guaranteed to not have dangling cell points, we can have a new mutating API for Path: - One method for appending a waypoint (implies adding a cell point too) - Oen method for appending a partial path, the last point of which is treated as a waypoint. This gives the following invariants for `Path`: 1. If there are no waypoints, then there are no cell points. 2. The first cell point in the path is a waypoint. 3. The last cell point in the path is a waypoint. 4. The waypoints are a subsequence of the cell points. Before this change, there we no invariants for `Path` - the cell points and waypoints could vary arbitrarily depending on the caller. The downside we still have is that the representation does not reflect this invariant. E.g., there is no way accurately pull the path back apart into its partial paths.
First of all, we copy the leader token before making any changes to the movement set. Otherwise we run the risk of modifying the leader partway and getting inconsistent results depending on iteration order. Next we pull the responsibility for derive the path out of `Token`. Previously, `Token.applyMove()` had to be aware of a bunch of information that is really specific to the renderer state, only to pass it along to `Path.derive()`. We now instead just make the renderer derive the path, then set it on the token. This also removes the need for `Token.applyMove()`. Finally, we push the calculation of the "cell offset" into `Path.derive()`. There is casework in the renderer and in `Path.derive()` that has to line up or else things don't work. So it is simpler to just have `Path.derive()` do the calculation in those cases where it is needed.
The calculation for how to derive a follower path is quite different now (and simpler). When both tokens are STG or both are not STG, the calculation is the same: simply apply the offset to all path points. This lets the follower's path have the exact same shape as the leader. When a non-STG token is following a STG leader, we reflect the waypoints of the leader in the follower path (offset, of course). This does not keep the full pathfinding shape when pathfinding around terrain, but at least the path is not arbitrarily chosen and does not suffer issues with zig-zagging. When a STG token is following a STG leader, we again reflect the waypoints. But in this case we have to snap each resulting point to the grid, and need to fill in partial paths between each waypoint. As before, this pathfinding is simple, i.e., it is not an A*-based algorithm.
This is not visibly different from before, though it ensures that STG followers have their position snapped to the grid, just as they would be had they been the leader.
Includes tests for all four derived path cases, and a test for waypoints in a self-crossing snap-to-grid path. The latter case used to include extraneous waypoints.
kwvanderlinde
changed the title
kwvanderlinde
changed the title
kwvanderlinde
changed the title
cwisniew
approved these changes
Jun 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Identify the Bug or Feature request
Fixes #4813
Description of the Change
Bug fix
A new approach is taken for deriving follower paths from leader paths. The general idea is that the waypoints in the follower path should follow the waypoints of the leader path, regardless of which tokens are STG or not. In most cases this is exact, but in the case of a STG token following a non-STG leader the waypoints are snapped to grid cells after being calculated. When both tokens are STG or both are not, the follower path will precisely follow the leader path even between waypoints (so pathfinding is accounted for). In the other cases, only the waypoints are considered, and intervening points may be interpolated.
Refactoring
There is a supporting refactor to
Pathto make the above idea solid.Pathnow has an invariant that makes sure the waypoint list and cell list make sense together:To do this, callers are only allowed to append to the path, not otherwise modify it. And they no longer add cells and waypoints separately. Instead they can either:
Only
SelectionSetandMeasureToolreally had a problem with that, and specifically for gridless paths. They were easily fixed by having them handle their own current point instead of hijacking thePathto do it for them.New unit tests are included that fully cover
Pathaside fromreadResolve().Possible Drawbacks
Serialized paths will not obey the new rules. This shouldn't really be an issue, but it does mean that if a user loads up an campaign, hit "Show Path" on a follower token, the paths they see won't match what they see if they tried the same drag that produced it originally.
Documentation Notes
Follower tokens try to follow the leader token as closely as possible when dragging.
Release Notes
This change is