Repository navigation
Conversation
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
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.
"GetStateCurvatures" and "ExtendedState.setPath" both turn the current way and the next ways into one list of positions in travel order, and each has its own copy of the same index loop to do it. This moves that into "GetStatePath", which reverses each way that's driven backward and appends each next way without its first node, since that's the junction already in the list. "GetStateCurvatures" now finds its merge and split junctions from the way lengths instead of tracking them inside the loop, and "setPath" just copies the positions out.
The output doesn't change, and it's 46 fewer lines.
Audit report
Why the output is identical
"Nodes.Slice()" returns a fresh copy, so reversing it and appending to it never touches a way's stored nodes. The old loop read each way forward or backward and skipped the first node of every next way, which is the same list "GetStatePath" builds. The junction before each next way is the last node of everything before it, so its index is the current way's length minus one, plus each earlier next way's length minus one. That's the index the old loop recorded when it moved on to the next way.
Testing
Not covered
The only inputs where the two differ are a current way with no nodes and a one-node next way in the middle of the chain, where the old loop indexed past the end and panicked. Neither came up in the replay.