Add stream creation tool for osu!standard editor

[OliBomby]

It introduces a new kind of hit object which is the stream. I didn't bother adding it to the legacy encoder system because that would break compatibility with stable, so saving/loading, undo's, and autoplay don't work properly on the stream hit object. I intentionally made it so the stream hit object gets converted to hit circles upon deselecting so no stream hit objects stay in the beatmap. This may be changed when beatmap serialization is updated.

osu.Game.Rulesets.Osu.Tests_AR3UI3Ft5f.mp4

• Add segment end completions list to SliderPath #21559
• Make PathControlPointVisualiser generic #21560
• Add timeline support for hit objects with multiple combos and multiple internal durations #21566

[minisbett]

omg this is insane

[LumpBloom7]

The extra drag points on the timeline blueprint may be useful for other HitObjects (such as custom ones) that may need auxiliary duration information.

Would be nice if such functionality is implemented in a manner that allows other HitObjects to make use of it, without needing to implement IHasStreamPath. (Maybe something like IHasAuxiliaryDurations?)

[OliBomby]

The extra drag points on the timeline blueprint may be useful for other HitObjects (such as custom ones) that may need auxiliary duration information.

Would be nice if such functionality is implemented in a manner that allows other HitObjects to make use of it, without needing to implement IHasStreamPath. (Maybe something like IHasAuxiliaryDurations?)

Good point, I think it shouldn't be hard to add this more generic interface. I'd call it IHasMultipleDurations since thats a bit more easy to understand.

[bdach]

I'm gonna sound like a debbie downer, but almost 40 changed files and a 2k+ line diffstat? Review-wise that's a tough sell. Is there no way to split this up more into a series of pulls to make reviewing this palatable?

[peppy]

I think these classes need to be in the editor namespace and not part of gameplay in any shape..

[OliBomby]

I'm gonna sound like a debbie downer, but almost 40 changed files and a 2k+ line diffstat? Review-wise that's a tough sell. Is there no way to split this up more into a series of pulls to make reviewing this palatable?

Maybe the part where I made the PathControlPointVisualizer generic can be split. That commit is responsible for most of the file changes. But is it really worth to go through the effort of splitting this PR just so review can be done in smaller steps.

[OliBomby]

A further PR can be made to turn these editor hit objects into their own thing, so they can automatically get removed or converted into hit objects before the beatmap starts gameplay.

[bdach]

But is it really worth to go through the effort of splitting this PR just so review can be done in smaller steps.

Oh absolutely. If I were more uptight about it, I'd say that splitting your changes is a requirement of having your changes upstreamed. I won't be that uptight - I've let huge diffs past before - but only when it was demonstrated that there is no sane way of splitting the change.

Put it this way: This PR covers a lot of untreaded ground, as far as the game is concerned. There haven't been any editor-specific hitobjects anywhere. I wish to examine the structure of doing so carefully and in isolation to menial changes like making a class generic, which is mostly tedium and noise.

Putting it all in one diff risks condemning your change to 100-review-comment-long review hell if/when a fundamental issue arises (exactly like the one above, re: namespace placement of the new objects). I assure you that I don't want that, and you probably don't, too.

It's fine to link branches with follow up work or queue dependency chains. This PR, though, is too difficult to review than it needs to be due to its sheer size. I can't explain it any better, it's just a thing you discover after you've read enough of these. I'd recommend doing self review of the diff and seeing how it feels, how long it takes, and whether you can mentally separate everything that was done here. Maybe it'll demonstrate it to some degree (although probably not, since you already know what was done, you're not entering blind).

[peppy]

From my perspective, I can say that this PR is going to require my review, and at this point I don't have even an estimate of when I'll get to it. Around 100 lines changed is the point where I am willing to jump in to a review blind and see what's involved; up to 400 lines changed is where that becomes a larger task. Anything over that is a special case.

Would very much appreciate if this can be further split out.

[OliBomby]

Okay I'll try to split up this PR into parts if I'm able to cherry-pick the right commits. There are a few menial changes that make sense to split, but splitting the new files which define the Stream object and its components is not really possible.

[peppy]

New files are fine to be large. It's the touching of existing files that comes with review overhead.