Merge pull request #10241 from peppy/editor-ternary-states

Rework ternary states to fix context menus not updating after already displayed

osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs

@@ -1,9 +1,10 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System;
 using System.Collections.Generic;
 using System.Linq;
+using osu.Framework.Allocation;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics.UserInterface;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Rulesets.Edit;
@@ -14,75 +15,80 @@ namespace osu.Game.Rulesets.Taiko.Edit
 {
     public class TaikoSelectionHandler : SelectionHandler
     {
+        private readonly Bindable<TernaryState> selectionRimState = new Bindable<TernaryState>();
+        private readonly Bindable<TernaryState> selectionStrongState = new Bindable<TernaryState>();
+
+        [BackgroundDependencyLoader]
+        private void load()
+        {
+            selectionStrongState.ValueChanged += state =>
+            {
+                switch (state.NewValue)
+                {
+                    case TernaryState.False:
+                        SetStrongState(false);
+                        break;
+
+                    case TernaryState.True:
+                        SetStrongState(true);
+                        break;
+                }
+            };
+
+            selectionRimState.ValueChanged += state =>
+            {
+                switch (state.NewValue)
+                {
+                    case TernaryState.False:
+                        SetRimState(false);
+                        break;
+
+                    case TernaryState.True:
+                        SetRimState(true);
+                        break;
+                }
+            };
+        }
+
+        public void SetStrongState(bool state)
+        {
+            var hits = SelectedHitObjects.OfType<Hit>();
+
+            ChangeHandler.BeginChange();
+
+            foreach (var h in hits)
+                h.IsStrong = state;
+
+            ChangeHandler.EndChange();
+        }
+
+        public void SetRimState(bool state)
+        {
+            var hits = SelectedHitObjects.OfType<Hit>();
+
+            ChangeHandler.BeginChange();
+
+            foreach (var h in hits)
+                h.Type = state ? HitType.Rim : HitType.Centre;
+
+            ChangeHandler.EndChange();
+        }
+
         protected override IEnumerable<MenuItem> GetContextMenuItemsForSelection(IEnumerable<SelectionBlueprint> selection)
         {
             if (selection.All(s => s.HitObject is Hit))
-            {
+                yield return new TernaryStateMenuItem("Rim") { State = { BindTarget = selectionRimState } };
-                var hits = selection.Select(s => s.HitObject).OfType<Hit>();
-
-                yield return new TernaryStateMenuItem("Rim", action: state =>
-                {
-                    ChangeHandler.BeginChange();
-
-                    foreach (var h in hits)
-                    {
-                        switch (state)
-                        {
-                            case TernaryState.True:
-                                h.Type = HitType.Rim;
-                                break;
-
-                            case TernaryState.False:
-                                h.Type = HitType.Centre;
-                                break;
-                        }
-                    }
-
-                    ChangeHandler.EndChange();
-                })
-                {
-                    State = { Value = getTernaryState(hits, h => h.Type == HitType.Rim) }
-                };
-            }
 
             if (selection.All(s => s.HitObject is TaikoHitObject))
-            {
+                yield return new TernaryStateMenuItem("Strong") { State = { BindTarget = selectionStrongState } };
-                var hits = selection.Select(s => s.HitObject).OfType<TaikoHitObject>();
-
-                yield return new TernaryStateMenuItem("Strong", action: state =>
-                {
-                    ChangeHandler.BeginChange();
-
-                    foreach (var h in hits)
-                    {
-                        switch (state)
-                        {
-                            case TernaryState.True:
-                                h.IsStrong = true;
-                                break;
-
-                            case TernaryState.False:
-                                h.IsStrong = false;
-                                break;
         }
 
-                        EditorBeatmap?.UpdateHitObject(h);
+        protected override void UpdateTernaryStates()
-                    }
-
-                    ChangeHandler.EndChange();
-                })
         {
-                    State = { Value = getTernaryState(hits, h => h.IsStrong) }
+            base.UpdateTernaryStates();
-                };
-            }
-        }
 
-        private TernaryState getTernaryState<T>(IEnumerable<T> selection, Func<T, bool> func)
+            selectionRimState.Value = GetStateFromSelection(SelectedHitObjects.OfType<Hit>(), h => h.Type == HitType.Rim);
-        {
+            selectionStrongState.Value = GetStateFromSelection(SelectedHitObjects.OfType<TaikoHitObject>(), h => h.IsStrong);
-            if (selection.Any(func))
-                return selection.All(func) ? TernaryState.True : TernaryState.Indeterminate;
-
-            return TernaryState.False;
         }
     }
 }

osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs

@@ -4,7 +4,9 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Humanizer;
 using osu.Framework.Allocation;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Cursor;
@@ -59,6 +61,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
         [BackgroundDependencyLoader]
         private void load(OsuColour colours)
         {
+            createStateBindables();
+
             InternalChild = content = new Container
             {
                 Children = new Drawable[]
@@ -308,6 +312,90 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         #endregion
 
+        #region Selection State
+
+        private readonly Bindable<TernaryState> selectionNewComboState = new Bindable<TernaryState>();
+
+        private readonly Dictionary<string, Bindable<TernaryState>> selectionSampleStates = new Dictionary<string, Bindable<TernaryState>>();
+
+        /// <summary>
+        /// Set up ternary state bindables and bind them to selection/hitobject changes (in both directions)
+        /// </summary>
+        private void createStateBindables()
+        {
+            // hit samples
+            var sampleTypes = new[] { HitSampleInfo.HIT_WHISTLE, HitSampleInfo.HIT_CLAP, HitSampleInfo.HIT_FINISH };
+
+            foreach (var sampleName in sampleTypes)
+            {
+                var bindable = new Bindable<TernaryState>
+                {
+                    Description = sampleName.Replace("hit", string.Empty).Titleize()
+                };
+
+                bindable.ValueChanged += state =>
+                {
+                    switch (state.NewValue)
+                    {
+                        case TernaryState.False:
+                            RemoveHitSample(sampleName);
+                            break;
+
+                        case TernaryState.True:
+                            AddHitSample(sampleName);
+                            break;
+                    }
+                };
+
+                selectionSampleStates[sampleName] = bindable;
+            }
+
+            // new combo
+            selectionNewComboState.ValueChanged += state =>
+            {
+                switch (state.NewValue)
+                {
+                    case TernaryState.False:
+                        SetNewCombo(false);
+                        break;
+
+                    case TernaryState.True:
+                        SetNewCombo(true);
+                        break;
+                }
+            };
+
+            // bring in updates from selection changes
+            EditorBeatmap.HitObjectUpdated += _ => UpdateTernaryStates();
+            EditorBeatmap.SelectedHitObjects.CollectionChanged += (sender, args) => UpdateTernaryStates();
+        }
+
+        /// <summary>
+        /// Called when context menu ternary states may need to be recalculated (selection changed or hitobject updated).
+        /// </summary>
+        protected virtual void UpdateTernaryStates()
+        {
+            selectionNewComboState.Value = GetStateFromSelection(SelectedHitObjects.OfType<IHasComboInformation>(), h => h.NewCombo);
+
+            foreach (var (sampleName, bindable) in selectionSampleStates)
+            {
+                bindable.Value = GetStateFromSelection(SelectedHitObjects, h => h.Samples.Any(s => s.Name == sampleName));
+            }
+        }
+
+        /// <summary>
+        /// Given a selection target and a function of truth, retrieve the correct ternary state for display.
+        /// </summary>
+        protected TernaryState GetStateFromSelection<T>(IEnumerable<T> selection, Func<T, bool> func)
+        {
+            if (selection.Any(func))
+                return selection.All(func) ? TernaryState.True : TernaryState.Indeterminate;
+
+            return TernaryState.False;
+        }
+
+        #endregion
+
         #region Context Menu
 
         public MenuItem[] ContextMenuItems
@@ -322,7 +410,9 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 items.AddRange(GetContextMenuItemsForSelection(selectedBlueprints));
 
                 if (selectedBlueprints.All(b => b.HitObject is IHasComboInformation))
-                    items.Add(createNewComboMenuItem());
+                {
+                    items.Add(new TernaryStateMenuItem("New combo") { State = { BindTarget = selectionNewComboState } });
+                }
 
                 if (selectedBlueprints.Count == 1)
                     items.AddRange(selectedBlueprints[0].ContextMenuItems);
@@ -331,12 +421,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 {
                     new OsuMenuItem("Sound")
                     {
-                        Items = new[]
+                        Items = selectionSampleStates.Select(kvp =>
-                        {
+                            new TernaryStateMenuItem(kvp.Value.Description) { State = { BindTarget = kvp.Value } }).ToArray()
-                            createHitSampleMenuItem("Whistle", HitSampleInfo.HIT_WHISTLE),
-                            createHitSampleMenuItem("Clap", HitSampleInfo.HIT_CLAP),
-                            createHitSampleMenuItem("Finish", HitSampleInfo.HIT_FINISH)
-                        }
                     },
                     new OsuMenuItem("Delete", MenuItemType.Destructive, deleteSelected),
                 });
@@ -353,76 +439,6 @@ namespace osu.Game.Screens.Edit.Compose.Components
         protected virtual IEnumerable<MenuItem> GetContextMenuItemsForSelection(IEnumerable<SelectionBlueprint> selection)
             => Enumerable.Empty<MenuItem>();
 
-        private MenuItem createNewComboMenuItem()
-        {
-            return new TernaryStateMenuItem("New combo", MenuItemType.Standard, setNewComboState)
-            {
-                State = { Value = getHitSampleState() }
-            };
-
-            void setNewComboState(TernaryState state)
-            {
-                switch (state)
-                {
-                    case TernaryState.False:
-                        SetNewCombo(false);
-                        break;
-
-                    case TernaryState.True:
-                        SetNewCombo(true);
-                        break;
-                }
-            }
-
-            TernaryState getHitSampleState()
-            {
-                int countExisting = selectedBlueprints.Select(b => (IHasComboInformation)b.HitObject).Count(h => h.NewCombo);
-
-                if (countExisting == 0)
-                    return TernaryState.False;
-
-                if (countExisting < SelectedHitObjects.Count())
-                    return TernaryState.Indeterminate;
-
-                return TernaryState.True;
-            }
-        }
-
-        private MenuItem createHitSampleMenuItem(string name, string sampleName)
-        {
-            return new TernaryStateMenuItem(name, MenuItemType.Standard, setHitSampleState)
-            {
-                State = { Value = getHitSampleState() }
-            };
-
-            void setHitSampleState(TernaryState state)
-            {
-                switch (state)
-                {
-                    case TernaryState.False:
-                        RemoveHitSample(sampleName);
-                        break;
-
-                    case TernaryState.True:
-                        AddHitSample(sampleName);
-                        break;
-                }
-            }
-
-            TernaryState getHitSampleState()
-            {
-                int countExisting = SelectedHitObjects.Count(h => h.Samples.Any(s => s.Name == sampleName));
-
-                if (countExisting == 0)
-                    return TernaryState.False;
-
-                if (countExisting < SelectedHitObjects.Count())
-                    return TernaryState.Indeterminate;
-
-                return TernaryState.True;
-            }
-        }
-
         #endregion
     }
 }