From e6174f195cf2fdfedf9c9a054172136e3b5b2efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 18 Feb 2025 12:06:42 +0100 Subject: [PATCH] Ensure `EditorBeatmap.PerformOnSelection()` marks objects in selection as updated 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. --- osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs | 6 ------ .../Edit/Compose/Components/EditorBlueprintContainer.cs | 8 +------- .../Edit/Compose/Components/EditorSelectionHandler.cs | 9 --------- osu.Game/Screens/Edit/EditorBeatmap.cs | 5 +++++ 4 files changed, 6 insertions(+), 22 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs b/osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs index be2a5ac144..364324087b 100644 --- a/osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs +++ b/osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs @@ -62,10 +62,7 @@ namespace osu.Game.Rulesets.Taiko.Edit if (h is not TaikoStrongableHitObject strongable) return; if (strongable.IsStrong != state) - { strongable.IsStrong = state; - EditorBeatmap.Update(strongable); - } }); } @@ -77,10 +74,7 @@ namespace osu.Game.Rulesets.Taiko.Edit EditorBeatmap.PerformOnSelection(h => { if (h is Hit taikoHit) - { taikoHit.Type = state ? HitType.Rim : HitType.Centre; - EditorBeatmap.Update(h); - } }); } diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs index e67644baaa..e8de1eaad9 100644 --- a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs @@ -81,13 +81,7 @@ namespace osu.Game.Screens.Edit.Compose.Components double offset = result.Time.Value - referenceTime; if (offset != 0) - { - Beatmap.PerformOnSelection(obj => - { - obj.StartTime += offset; - Beatmap.Update(obj); - }); - } + Beatmap.PerformOnSelection(obj => obj.StartTime += offset); } protected override void AddBlueprintFor(HitObject item) diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs b/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs index cd6e25734a..f9e7ef6df8 100644 --- a/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs +++ b/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs @@ -355,8 +355,6 @@ namespace osu.Game.Screens.Edit.Compose.Components for (int i = 0; i < hasRepeats.NodeSamples.Count; ++i) hasRepeats.NodeSamples[i] = hasRepeats.NodeSamples[i].Select(s => s.Name == HitSampleInfo.HIT_NORMAL ? s.With(newBank: bankName) : s).ToList(); } - - EditorBeatmap.Update(h); }); } @@ -390,8 +388,6 @@ namespace osu.Game.Screens.Edit.Compose.Components hasRepeats.NodeSamples[i] = hasRepeats.NodeSamples[i].Select(s => s.Name != HitSampleInfo.HIT_NORMAL ? bankName == HIT_BANK_AUTO ? s.With(newBank: normalBank, newEditorAutoBank: true) : s.With(newBank: bankName, newEditorAutoBank: false) : s).ToList(); } } - - EditorBeatmap.Update(h); }); } @@ -439,8 +435,6 @@ namespace osu.Game.Screens.Edit.Compose.Components node.Add(hitSample); } } - - EditorBeatmap.Update(h); }); } @@ -462,8 +456,6 @@ namespace osu.Game.Screens.Edit.Compose.Components for (int i = 0; i < hasRepeats.NodeSamples.Count; ++i) hasRepeats.NodeSamples[i] = hasRepeats.NodeSamples[i].Where(s => s.Name != sampleName).ToList(); } - - EditorBeatmap.Update(h); }); } @@ -484,7 +476,6 @@ namespace osu.Game.Screens.Edit.Compose.Components if (comboInfo == null || comboInfo.NewCombo == state) return; comboInfo.NewCombo = state; - EditorBeatmap.Update(h); }); } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 44f9646889..254336e963 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -312,8 +312,13 @@ namespace osu.Game.Screens.Edit return; BeginChange(); + foreach (var h in SelectedHitObjects) + { action(h); + Update(h); + } + EndChange(); }