Fix dragging a hold note end outside playfield bypassing snapping#37025
Fix dragging a hold note end outside playfield bypassing snapping#37025peppy wants to merge 6 commits intoppy:masterfrom
Conversation
peppy
changed the title
At first I considered passing the `SnapResult` into subsequent snap operations to gain persistence of playfield (stored within), but this solution seems simpler. We can assume that osu!mania is never going to have overlapping columns to the point that basing the editor considerations on *closest* column rather than *containing* column would become an issue. As far as I can tell. Closes ppy#36966.
peppy
force-pushed
the
fix-mania-editor-drag-outside-playfield
branch
from
459433f to
71ff44b
Compare
|
(just merging master because inspectcode is like hallucinating errors that don't exist) |
|
^ didn't help... I'm probably just going to get out the inspectcode changes and hope they help with whatever this is... On another note, not sure how to feel about the change in behaviour this brings in placement mode. Previously you couldn't place objects when the mouse cursor was outside of playfield in mania (though it was a little broken still): Screen.Recording.2026-03-18.at.10.02.21.movNow you can do this and the placement isn't even visible until drag: Screen.Recording.2026-03-18.at.10.03.07.movProbably shouldn't be allowed, or if it should, it should at least be shown? |
|
I've fixed the issue you pointed out with placement. Note that some tests will require updates because of this change (likely the DI part). Can you check 30d54d3 before I do this to make sure you agree with the direction? |
| /// If <c>false</c> is returned and a commit is attempted, the blueprint will be destroyed instead. | ||
| /// </remarks> | ||
| protected virtual bool IsValidForPlacement => true; | ||
| protected virtual bool IsValidForPlacement => PlacementActive != PlacementState.Waiting || hitObjectComposer.CursorInPlacementArea; |
There was a problem hiding this comment.
On a superficial check this change looks potentially dicey. The conditions look right but I dunno that enforcing them at this deep a level (in terms of inheritance hierarchy, I mean) is correct.
The rest of the commit seems fine. Not sure whether you expected me to run this, all I did was an ocular patdown of the diff.
There was a problem hiding this comment.
Yeah, pretty much fine with that.
2 participants
At first I considered passing the
SnapResultinto subsequent snap operations to gain persistence of playfield (stored within), but this solution seems simpler.We can assume that osu!mania is never going to have overlapping columns to the point that basing the editor considerations on closest column rather than containing column would become an issue. As far as I can tell.
Closes #36966.