Fix hit animation not synchronizing when editing hit objects

[OliBomby]

This should keep the JudgementResult.RawTime synchronized with the end time because the DefaultsApplied event should always fire whenever a hitobject is changed in the editor.

It seems that the StartTime based method of synchronizing the state was also not able to preserve the invariant of monotone time in the judgedEntries, so something needed to be done to re-sort these entries. To do this I need to notify Playfield of any hit object updates, so HitObjectContainer gets a new event to propagate these events.

This PR does not address the issue of the monotone time invariant in the judgedEntries being violated. This issue will be fixed in another PR.

osu.Game.Rulesets.Osu.Tests_D4PXYerjWU.mp4

closes #22698

[bdach]

Probably relevant issue thread: #22698

I've already proposed hooking into HitObjectUpdated() to fix this sort of thing, but the solution was deemed too ugly to live. So not sure how this is gonna fare.

[OliBomby]

Probably relevant issue thread: #22698

I've already proposed hooking into HitObjectUpdated() to fix this sort of thing, but the solution was deemed too ugly to live. So not sure how this is gonna fare.

I saw your proposed solution. Mine is a bit different in that it doesn't reset all the results all the time. I think using DefaultsApplied to detect changes is the right approach, because it is the only thing that signals any change to the hitobject and it allows us to update judgment results for only the hit object that changed.

[peppy]

@bdach what's your state on this one? Curious if you've already reviewed and are mostly on board with it, or if you're first waiting for my response?

if the latter, we can probably back-burner it until after scoring, but if you've already put thought into this, from a quick check I would probably be okay with the direction (probably with a touch more inline commenting). My main concern is the binding to every HitObject's OnDefaultsApplied from a performance perspective, but it's in the "good" direction (many objects bound to single event, not the inverse) so probably okay.

[bdach]

I was waiting to see if you're willing to even entertain this diff, since it's not very far removed from the earlier proposal of mine that you said was too ugly to live.

[peppy]

I prefer your version if it works since both are sub-optimal solutions, and at least yours is more isolated. Shall we go with that?

[OliBomby]

Bdach's solution doesnt work because it doesn't address the issue of monotone time in the 'judgedEntries'. My fix for that is the source of all the not-so-isolated changes.

The part of my diff that does the same thing as bdach's fix is in 'DrawableHitObject'.

[bdach]

It's not clear from the description of this PR what that breaks, exactly. And I'm not sure fixing that warrants invasive changes, there's probably a way to make them not so. Likely by usage of SortedList or similar.

[peppy]

If that's the case I'm not sure we'll get to this before the scoring work. If you can do anything to better isolate this as a temporary workaround it may be possible to get it in. See if you can come up with anything (even putting in a #region might help a bit – just want to make sure anyone coming across this code knows precisely why it's there, and is aware that it can be refactored / removed if surrounding stuff works better).

[OliBomby]

I'll split off the fix for monotone time invariant to a separate PR since that seems to spark some discussion.

The remaining fix should be small in size, so hope that can get in in time.

[bdach]

This sounds circular. What are the "changes in state" at the start there? Hitobject property changes? Not literal changes to DrawableHitObject.State?

[OliBomby]

The "changes in state" refers to state of the HitObject

[bdach]

Updated comment to be clearer in 5be5335.