This is the secondary cause of https://github.com/ppy/osu/issues/28577,
because you could do the following:
- Have a break autogenerate itself
- Adjust either end of it to make it mark itself as manually-adjusted
- Remove all objects before or after said break
to end up in a state wherein there are no objects before or after a
break.
The direct fix is still correct because it is still technically possible
to end up in a state wherein a break is before or after all objects
(obvious one is manual `.osu` editing), but this behaviour is also
undesirable for the autogeneration logic.
This was reported in https://github.com/ppy/osu/pull/28474, albeit the
code changes proposed there did not fix the issue at all.
See 8b6385f7d0 for demonstration of the
crash scenario. Basically what is happening there is:
- The starting premise is that there is a spinner placement active.
- At this time, a drag selection is started via the timeline.
- Once the drag selection finds at least one suitable object to select,
it mutates `SelectedItems`.
- When selection changes for any reason, the `HitObjectComposer`
decides to switch to the "select" tool, regardless of why
the selection changed.
- Changing the active tool causes the current placement - if any -
to be committed, which mutates the beatmap.
- Back at the drag box selection code, this causes a "collection
modified when enumerating" exception.
The proposed fix here is to eagerly commit active placement - if any -
when drag selection is initiated via the timeline, which avoids this
issue. This also appears to vaguely match stable behaviour and is sort
of consistent with the logic of committing any outstanding changes upon
switching to the selection tool.
This sort of thing has been showing up on flyte designs more and more
so I want to start using it more over that rather ugly "overlined" text
that's there on multiplayer screens right now.
Closes https://github.com/ppy/osu/issues/25426.
Different approach to prior ones, this just disables the relevant
actions when something related to save/export is going on. Still ends up
being convoluted because many things you wouldn't expect to touch save
do touch save, so it's not just a concern between export and save
specifically.
Closes https://github.com/ppy/osu/issues/28369.
The reporter of the issue was incorrect; it's not the beat snap grid
that is causing the problem, it's something far stupider than that.
When the current selection changes,
`EditorSelectionHandler.UpdateTernaryStates()` is supposed to update the
state of ternary bindables to reflect the reality of the current
selection. This in turn will fire bindable change callbacks for said
ternary toggles, which heavily use `EditorBeatmap.PerformOnSelection()`.
The thing about that method is that it will attempt to check whether any
changes were actually made to avoid producing empty undo states, *but*
to do this, it must *serialise out the entire beatmap to a stream* and
then *binary equality check that* to determine whether any changes were
actually made:
7b14c77e43/osu.Game/Screens/Edit/EditorChangeHandler.cs (L65-L69)
As goes without saying, this is very expensive and unnecessary, which
leads to stuff like keeping a selection box active while a taiko beatmap
is playing under it dog slow. So to attempt to mitigate that, add
precondition checks to every single ternary callback of this sort to
avoid this serialisation overhead.
And yes, those precondition checks use linq, and that is *still* faster
than not having them.
