osu icon indicating copy to clipboard operation
osu copied to clipboard

Add breathing room to seek back between control points in editor

Open TaterToes opened this issue 1 year ago • 2 comments

closes #28885 (and #30070)

gives 450 ms to seek back after last control point in editor.

TaterToes avatar Oct 17 '24 20:10 TaterToes

Screenshot_20241018_104003_Chrome What does this mean?

TaterToes avatar Oct 18 '24 14:10 TaterToes

Not anything you need to concern yourself with. That problem is already fixed in master.

bdach avatar Oct 18 '24 14:10 bdach

Stable logic for this is

            if (AudioEngine.ActiveInheritedTimingPointIndex < 0) return;

            if (AudioEngine.Time - AudioEngine.ControlPoints[AudioEngine.ActiveInheritedTimingPointIndex].Offset > 1000)
            {
                AudioEngine.SeekTo((int)AudioEngine.ControlPoints[AudioEngine.ActiveInheritedTimingPointIndex].Offset);
                return;
            }

            if (AudioEngine.ActiveInheritedTimingPointIndex == 0)
                return;

            AudioEngine.SeekTo((int)AudioEngine.ControlPoints[AudioEngine.ActiveInheritedTimingPointIndex - 1].Offset);

(https://github.com/peppy/osu-stable-reference/blob/38bbc10a53c78d83c9f04a04f7bf4818505cd313/osu!/GameModes/Edit/Editor.cs#L522-L536)

In particular note:

  • the different lenience interval (1000ms) - I'd probably apply this change
  • the lack of dispensation for when the track is running, which is present in this PR - I think checking track running state is fine, so I wouldn't want any changes with respect to this

(Side note but the stable implementation is bugged - if you have a non-inherited point and inherited point on the same timestamp, e.g. a "yellow line", you won't be able to seek back beyond it. Not relevant in this context anyhow.)

@peppy obligatory summon for UX opinion on desired behaviour

bdach avatar Oct 22 '24 08:10 bdach

I really do question 8 commits for 10 LoC changes and it's still failing basic formatting requirements, but let's overlook that for now 🙈

peppy avatar Oct 22 '24 09:10 peppy

I really do question 8 commits for 10 LoC changes and it's still failing basic formatting requirements, but let's overlook that for now 🙈

I've given up on even remarking on that sort of thing at this point because it helps nothing and only looks snarky.

bdach avatar Oct 22 '24 09:10 bdach

I guess in an attempt to make it non-snarky, @TaterToes consider not PRing until you're done with making changes, then squash your changes down to less commits if you want to seem more competent 😅

peppy avatar Oct 22 '24 09:10 peppy