Fix various shortcomings in juice stream selection blueprint #28999
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
- Use same hover state - Use shift-right click for quick delete rather than shift-left click
bdach
commented
Jul 22, 2024
Comment on lines
+161
to
+168
| addAddVertexSteps(500, 180); | ||
| addVertexCheckStep(3, 1, 500, 180); | ||
|
|
||
| addAddVertexSteps(90, 200); | ||
| addVertexCheckStep(4, 1, times[0], positions[0]); | ||
|
|
||
| addAddVertexSteps(750, 180); | ||
| addVertexCheckStep(5, 4, 750, 180); | ||
| addAddVertexSteps(750, 200); | ||
| addVertexCheckStep(5, 4, 750, 200); |
There was a problem hiding this comment.
If these test "fixes" appear arbitrary - that is because they are. They would fail after the rest of the changes, and as far as I can tell that is just a manifestation of #28915 that wasn't there before due to the lack of inline UpdateHitObjectFromPath() calls. I would ask to look away from this for now while I attempt to figure out what is going on over there (it is not easy, 99% chance it is floating point hell).
| double time = Math.Max(0, PositionToTime(relativePosition.Y)); | ||
| int index = AddVertex(time, relativePosition.X); | ||
| UpdateHitObjectFromPath(juiceStream); |
There was a problem hiding this comment.
Calls are required to actually make undo/redo work, because without this call, nothing in the actual beatmap changes, and changeHandler?.EndChange() ends up being a no-op.
1 task
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.
I was looking at #28915 and attempting to see what was going on but the current UX of juice stream selection was tripping me up so I decided to fix first.