catch editor JuiceStream start fruit offset to the right after switching from another placement tool #31423
Assignees
Labels
Comments
WangleLine
changed the title
|
From what I can remember, at some point it didn't happen before, so possibly some code broke this behavior? I have experienced this at least a month ago too. |
bdach
added a commit
to bdach/osu
that referenced
this issue
Jan 8, 2025
- Closes ppy#31423. - Regressed in ppy#30411. Admittedly, I don't completely understand all of the pieces here, because code quality of this placement blueprint code is ALL-CAPS ATROCIOUS, but I believe the failure mode to be something along the lines of: - User activates juice stream tool, blueprint gets created in initial state. It reads in a mouse position far outside of the playfield, and sets internal positioning appropriately. - When the user moves the mouse into the bounds of the playfield, some positions update (the ones inside `UpdateTimeAndPosition()`, but the fruit markers are for *nested* objects, and `updateHitObjectFromPath()` is responsible for updating those... however, it only fires if the `editablePath.PathId` changes, which it won't here, because there is only one path vertex until the user commits the starting point of the juice stream and it's always at (0,0). - Therefore the position of the starting fruit marker remains bogus until left click, at which point the path changes and everything returns to *relative* sanity. An alternative solution that shows this hypothesis to be true follows in the diff below: diff --cc osu.Game.Rulesets.Catch/Edit/Blueprints/JuiceStreamPlacementBlueprint.cs index 7b57dac36e,7b57dac36e..21cc260462 --- a/osu.Game.Rulesets.Catch/Edit/Blueprints/JuiceStreamPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Catch/Edit/Blueprints/JuiceStreamPlacementBlueprint.cs @@@ -88,10 -88,10 +88,9 @@@ namespace osu.Game.Rulesets.Catch.Edit. switch (PlacementActive) { case PlacementState.Waiting: -- if (!(result.Time is double snappedTime)) return; -- HitObject.OriginalX = ToLocalSpace(result.ScreenSpacePosition).X; -- HitObject.StartTime = snappedTime; ++ if (result.Time is double snappedTime) ++ HitObject.StartTime = snappedTime; break; case PlacementState.Active: @@@ -107,21 -107,21 +106,13 @@@ Vector2 startPosition = CatchHitObjectUtils.GetStartPosition(HitObjectContainer, HitObject); editablePath.Position = nestedOutlineContainer.Position = scrollingPath.Position = startPosition; -- updateHitObjectFromPath(); -- } -- -- private void updateHitObjectFromPath() -- { -- if (lastEditablePathId == editablePath.PathId) -- return; ++ if (lastEditablePathId != editablePath.PathId) ++ editablePath.UpdateHitObjectFromPath(HitObject); ++ lastEditablePathId = editablePath.PathId; -- editablePath.UpdateHitObjectFromPath(HitObject); ApplyDefaultsToHitObject(); -- scrollingPath.UpdatePathFrom(HitObjectContainer, HitObject); nestedOutlineContainer.UpdateNestedObjectsFrom(HitObjectContainer, HitObject); -- -- lastEditablePathId = editablePath.PathId; } private double positionToTime(float relativeYPosition) The patch essentially relies on inlining the broken method and only guarding the relevant part of processing behind the path version check (which is actually updating the path). Everything else that can touch positions of nesteds (like default application, and the drawable piece updates) is allowed to happen unconditionally. On one side, that patch looks more proper, but also does more processing, and is less safe, so I kinda just went with the easier option?
TheDark98
pushed a commit
to TheDark98/osu
that referenced
this issue
Jan 14, 2025
- Closes ppy#31423. - Regressed in ppy#30411. Admittedly, I don't completely understand all of the pieces here, because code quality of this placement blueprint code is ALL-CAPS ATROCIOUS, but I believe the failure mode to be something along the lines of: - User activates juice stream tool, blueprint gets created in initial state. It reads in a mouse position far outside of the playfield, and sets internal positioning appropriately. - When the user moves the mouse into the bounds of the playfield, some positions update (the ones inside `UpdateTimeAndPosition()`, but the fruit markers are for *nested* objects, and `updateHitObjectFromPath()` is responsible for updating those... however, it only fires if the `editablePath.PathId` changes, which it won't here, because there is only one path vertex until the user commits the starting point of the juice stream and it's always at (0,0). - Therefore the position of the starting fruit marker remains bogus until left click, at which point the path changes and everything returns to *relative* sanity. The solution essentially relies on inlining the broken method and only guarding the relevant part of processing behind the path version check (which is actually updating the path). Everything else that can touch positions of nesteds (like default application, and the drawable piece updates) is allowed to happen unconditionally.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Type
Cosmetic
Bug description
Select Fruit tool, then switch to JuiceStream tool, and the initial fruit of the stream will be visually offset for the first stream.
This does not affect actual placement. It's only the bit of UI that shows where the stream will start.
Screenshots or videos
catch.stream.UI.bug.mp4
Version
2025.101.0-lazer
Logs
compressed-logs.zip
The text was updated successfully, but these errors were encountered: