Closes https://github.com/ppy/osu/issues/32052.
Sooooo... this is going to be a rant...
To understand why this is going to require a rant, dear reader, please
do the following:
1. Read the issue thread and follow the reproduction scenario (download
map linked, fire up autoplay, seek near end, wait for results, hear
the sample spam).
2. Now exit out to song select, *hide the toolbar*, and attempt
reproducing the issue again.
3. Depending on ambient mood, laugh or cry.
Now, *why on earth* would the *TOOLBAR* have any bearing on anything?
Well, the chain of failure is something like this:
- The toolbar hides for the duration of gameplay, naturally.
- When progressing to results, the toolbar gets automatically unhidden.
- This triggers invalidations on `ScrollingHitObjectContainer`. I'm not
precisely sure which property it is that triggers the invalidations,
but one clearly does. It may be position or size or whichever.
- When the invalidation is triggered on `layoutCache`, the next
`Update()` call is going to recompute lifetimes for ALL hitobject
entries.
- In case of swells, it happens that the calculated lifetime end of the
swell is larger than what it actually ended up being determined as at
the instant of judging the swell, and thus, the swell is *resurrected*,
reassigned a DHO, and the DHO calls `UpdateState()` and plays the
sample again despite the `samplePlayed` flag in `LegacySwell`, because
all of that is ephemeral state that does not survive a hitobject
getting resurrected.
Now I *could* just fix this locally to the swell, maybe, by having some
time lenience check, but the fact that hitobjects can be resurrected by
the *toolbar* appearing, of all possible causes in the world, feels
just completely wrong. So I'm adding a local check in SHOC to not
overwrite lifetime ends of judged object entries.
The reason why I'm making that check specific to end time is that I can
see valid reasons why you would want to recompute lifetime *start* even
on a judged object (such as playfield geometry changing in a significant
way). I can't think of a valid reason to do that to lifetime *end*.
The lack of this bricks chat completely due to newtonsoft
deserialisation errors:
2025-02-24 08:32:58 [verbose]: Processing response from https://dev.ppy.sh/api/v2/chat/updates failed with Newtonsoft.Json.JsonSerializationException: Error converting value "TEAM" to type 'osu.Game.Online.Chat.ChannelType'. Path 'presence[39].type', line 1, position 13765.
2025-02-24 08:32:58 [verbose]: ---> System.ArgumentException: Requested value 'TEAM' was not found.
2025-02-24 08:32:58 [verbose]: at Newtonsoft.Json.Utilities.EnumUtils.ParseEnum(Type enumType, NamingStrategy namingStrategy, String value, Boolean disallowNumber)
2025-02-24 08:32:58 [verbose]: at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
It's no longer possible to just assume that using the ambient
`WorkingBeatmap` is gonna work.
Bit dodgy but seems to work and also I'd hope that `WorkingBeatmapCache`
makes this not overly taxing. If there are concerns this can probably be
an async load or something.
Closes https://github.com/ppy/osu/issues/28791.
The reason why nudging was not changing hyperdash state in catch was
that `EditorBeatmap.Update()` was not being called on the objects that
were being modified, therefore postprocessing was not performed,
therefore hyperdash state was not being recomputed.
Looking at the usage sites of `EditorBeatmap.PerformOnSelection()`,
about two-thirds of callers called `Update()` themselves on the objects
they mutated, and the rest didn't. I'd say that's the failure of the
abstraction and it should be `PerformOnSelection()`'s responsibility to
call `Update()` there. Yes in some of the cases here this will cause
extraneous calls that weren't done before, but the method is already
heavily disclaimed as 'expensive', so I'd say usability should come
first.
Flushes are assumed to have already come from a definitive state change
(read: disconnection). Allowing the exceptions that come from failing
the flushed requests to trigger the `Failing` code paths makes
completely incorrect behaviour possible.
Because it'll fail anyway - there is either no username or no password.
The reason why this is important is that the block was also setting API
state to `Connecting`.
When API is in `RequiresSecondFactorAuth` state, `attemptConnect()` is
called over and over in a loop, with no sleeping, which means that the
scheduler accumulates hundreds of thousands of these delegates. Sure you
could add a sleep in there maybe, but it seems pretty wasteful to have
the `localUser.IsDefault` guard *inside* the schedule anyway, so this is
what I opted for.
