Skip to content

Replace hit objects when placing at same time in editor#37485

Merged
peppy merged 5 commits intoppy:masterfrom
Hiviexd:editor/object-replace-ux-2
May 8, 2026
Merged

Replace hit objects when placing at same time in editor#37485
peppy merged 5 commits intoppy:masterfrom
Hiviexd:editor/object-replace-ux-2

Conversation

@Hiviexd
Copy link
Copy Markdown
Member

@Hiviexd Hiviexd commented Apr 22, 2026

Matches stable and significantly improves UX when mapping. In addition, current behavior makes it too easy to place stacked objects which is something we should not encourage.

@bdach bdach self-requested a review May 7, 2026 10:30
Copy link
Copy Markdown
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to somewhat prioritise this because it is a recurrent (stable) editor user complaint.

That said, I'm uncertain of some of the decisions made here.

  • The new behaviour is applied unconditionally without a toggle. That's probably fine by me, but I'm not sure that everyone else would agree.
  • More importantly, there is zero visual guidance indicating that an object will get deleted via this flow. Unless you're accustomed to stable editor and know how this works, there's no way for you to know what's going to happen. I'd see either a red tint, a red border, or a fade-out effect applied to the object that is getting replaced via this flow (both on the playfield, and on the timeline).

@peppy do you have anything to say on either of the above points?

There's not much to look at here code-wise. I left one comment but it's of little weight.

Comment thread osu.Game/Rulesets/Edit/HitObjectPlacementBlueprint.cs Outdated
@peppy
Copy link
Copy Markdown
Member

peppy commented May 8, 2026

@bdach I'm okay with both points. I think it's expected behaviour for rulesets where placing multiple notes at the same time value is not valid.

Copy link
Copy Markdown
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-resolved review comment and added a test for mania behaviour because there wasn't one.

@peppy Your comment above is worded a bit ambiguously but I'm interpreting it such that you're fine with how this works as written. To that end I'll leave this hanging for a bit and merge eventually just in case I interpreted wrong.

Copy link
Copy Markdown
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is good

@cihe13375
Copy link
Copy Markdown

I understand that this change logically makes sense, but I do sometimes "exploit" the old behavior, e.g. in making hitsound difficulties. In the example below, each location on the playfield represents a type of sample (whistle, cymbal, kick, snare), and there are dedicated tools for assigning samples based on locations and copying them to other difficulties.
aaaaa

Also I am not sure if 2ms leniency is enough. In theory two notes at the same tick can be at most 3.99999999999999ms apart (since unsnaps <2ms are considered "negligible").

@bdach
Copy link
Copy Markdown
Collaborator

bdach commented May 8, 2026

I understand that this change logically makes sense, but I do sometimes "exploit" the old behavior, e.g. in making hitsound difficulties. In the example below, each location on the playfield represents a type of sample (whistle, cymbal, kick, snare), and there are dedicated tools for assigning samples based on locations and copying them to other difficulties.

This is so hacky I'm not even really wanting to entertain it. Hopefully we can provide a flow for this that's not made of loose sticks and string.

Also I am not sure if 2ms leniency is enough. In theory two notes at the same tick can be at most 3.99999999999999ms apart (since unsnaps <2ms are considered "negligible").

if (Math.Abs(unsnap) >= UNSNAP_MS_THRESHOLD)
yield return new IssueTemplateLargeUnsnap(this).Create(hitobject, unsnap, time, postfix);
else if (Math.Abs(unsnap) >= 1)
yield return new IssueTemplateSmallUnsnap(this).Create(hitobject, unsnap, time, postfix);

public const double UNSNAP_MS_THRESHOLD = 2;

2 seems correct / sufficient to me here given that placement tools are going to be using the same snapping that the above code checks for snapping tolerance, with no room for perturbations.

@peppy
Copy link
Copy Markdown
Member

peppy commented May 8, 2026

Agree with reply here. No changes required.

@peppy peppy merged commit 4d88c14 into ppy:master May 8, 2026
8 of 10 checks passed
@Hiviexd Hiviexd deleted the editor/object-replace-ux-2 branch May 8, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants