Replace FormSliderBar.Instantaneous with TransferValueOnCommit

Rather than control the propagation of the value between the slider and
the textbox, add a property that controls the propagation of the value
between the bindables inside the form control to external bindables.
This will help alleviate issues where the external bindable update
incurs overheads due to having heavy change callbacks attached.

osu.Game.Tests/Visual/UserInterface/TestSceneFormControls.cs

@@ -72,7 +72,7 @@ public TestSceneFormControls()
                         },
                         new FormSliderBar<float>
                         {
-                            Caption = "Instantaneous slider",
+                            Caption = "Slider",
                             Current = new BindableFloat
                             {
                                 MinValue = 0,
@@ -82,19 +82,6 @@ public TestSceneFormControls()
                             },
                             TabbableContentContainer = this,
                         },
-                        new FormSliderBar<float>
-                        {
-                            Caption = "Non-instantaneous slider",
-                            Current = new BindableFloat
-                            {
-                                MinValue = 0,
-                                MaxValue = 10,
-                                Value = 5,
-                                Precision = 0.1f,
-                            },
-                            Instantaneous = false,
-                            TabbableContentContainer = this,
-                        },
                         new FormEnumDropdown<CountdownType>
                         {
                             Caption = EditorSetupStrings.EnableCountdown,

osu.Game.Tests/Visual/UserInterface/TestSceneFormSliderBar.cs

@@ -0,0 +1,63 @@
+// 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 NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Extensions.ObjectExtensions;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Game.Graphics.Sprites;
+using osu.Game.Graphics.UserInterfaceV2;
+using osu.Game.Overlays;
+using osuTK;
+
+namespace osu.Game.Tests.Visual.UserInterface
+{
+    public partial class TestSceneFormSliderBar : OsuTestScene
+    {
+        [Cached]
+        private OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Aquamarine);
+
+        [Test]
+        public void TestTransferValueOnCommit()
+        {
+            OsuSpriteText text;
+            FormSliderBar<float> slider = null!;
+
+            AddStep("create content", () =>
+            {
+                Child = new FillFlowContainer
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    Width = 0.5f,
+                    Direction = FillDirection.Vertical,
+                    Spacing = new Vector2(10),
+                    Children = new Drawable[]
+                    {
+                        text = new OsuSpriteText(),
+                        slider = new FormSliderBar<float>
+                        {
+                            Caption = "Slider",
+                            Current = new BindableFloat
+                            {
+                                MinValue = 0,
+                                MaxValue = 10,
+                                Precision = 0.1f,
+                                Default = 5f,
+                            }
+                        },
+                    }
+                };
+                slider.Current.BindValueChanged(_ => text.Text = $"Current value is: {slider.Current.Value}", true);
+            });
+            AddToggleStep("toggle transfer value on commit", b =>
+            {
+                if (slider.IsNotNull())
+                    slider.TransferValueOnCommit = b;
+            });
+        }
+    }
+}

osu.Game/Graphics/UserInterfaceV2/FormSliderBar.cs

@@ -28,26 +28,22 @@ public partial class FormSliderBar<T> : CompositeDrawable, IHasCurrentValue<T>
         public Bindable<T> Current
         {
             get => current.Current;
-            set => current.Current = value;
+            set
+            {
+                current.Current = value;
+                currentNumberInstantaneous.Default = current.Default;
+            }
         }
 
-        private bool instantaneous = true;
+        private readonly BindableNumberWithCurrent<T> current = new BindableNumberWithCurrent<T>();
+
+        private readonly BindableNumber<T> currentNumberInstantaneous = new BindableNumber<T>();
 
         /// <summary>
-        /// Whether changes to the slider should instantaneously transfer to the text box (and vice versa).
-        /// If <see langword="false"/>, the transfer will happen on text box commit (explicit, or implicit via focus loss), or on slider drag end.
+        /// Whether changes to the value should instantaneously transfer to outside bindables.
+        /// If <see langword="false"/>, the transfer will happen on text box commit (explicit, or implicit via focus loss), or on slider commit.
         /// </summary>
-        public bool Instantaneous
-        {
-            get => instantaneous;
-            set
-            {
-                instantaneous = value;
-
-                if (slider.IsNotNull())
-                    slider.TransferValueOnCommit = !instantaneous;
-            }
-        }
+        public bool TransferValueOnCommit { get; set; }
 
         private CompositeDrawable? tabbableContentContainer;
 
@@ -62,8 +58,6 @@ public CompositeDrawable? TabbableContentContainer
             }
         }
 
-        private readonly BindableNumberWithCurrent<T> current = new BindableNumberWithCurrent<T>();
-
         /// <summary>
         /// Caption describing this slider bar, displayed on top of the controls.
         /// </summary>
@@ -147,8 +141,8 @@ private void load(OsuColour colours, OsuGame? game)
                             Origin = Anchor.CentreRight,
                             RelativeSizeAxes = Axes.X,
                             Width = 0.5f,
-                            Current = Current,
-                            TransferValueOnCommit = !instantaneous,
+                            Current = currentNumberInstantaneous,
+                            OnCommit = () => current.Value = currentNumberInstantaneous.Value,
                         }
                     },
                 },
@@ -170,9 +164,28 @@ protected override void LoadComplete()
 
             slider.IsDragging.BindValueChanged(_ => updateState());
 
+            current.ValueChanged += e => currentNumberInstantaneous.Value = e.NewValue;
+            current.MinValueChanged += v => currentNumberInstantaneous.MinValue = v;
+            current.MaxValueChanged += v => currentNumberInstantaneous.MaxValue = v;
+            current.PrecisionChanged += v => currentNumberInstantaneous.Precision = v;
+            current.DisabledChanged += disabled =>
+            {
+                if (disabled)
+                {
+                    // revert any changes before disabling to make sure we are in a consistent state.
+                    currentNumberInstantaneous.Value = current.Value;
+                }
+
+                currentNumberInstantaneous.Disabled = disabled;
+            };
+
+            current.CopyTo(currentNumberInstantaneous);
             currentLanguage.BindValueChanged(_ => Schedule(updateValueDisplay));
-            current.BindValueChanged(_ =>
+            currentNumberInstantaneous.BindValueChanged(e =>
             {
+                if (!TransferValueOnCommit)
+                    current.Value = e.NewValue;
+
                 updateState();
                 updateValueDisplay();
             }, true);
@@ -182,17 +195,15 @@ protected override void LoadComplete()
 
         private void textChanged(ValueChangedEvent<string> change)
         {
-            if (!instantaneous) return;
-
             tryUpdateSliderFromTextBox();
         }
 
         private void textCommitted(TextBox t, bool isNew)
         {
             tryUpdateSliderFromTextBox();
-
             // If the attempted update above failed, restore text box to match the slider.
-            Current.TriggerChange();
+            currentNumberInstantaneous.TriggerChange();
+            current.Value = currentNumberInstantaneous.Value;
 
             flashLayer.Colour = ColourInfo.GradientVertical(colourProvider.Dark2.Opacity(0), colourProvider.Dark2);
             flashLayer.FadeOutFromOne(800, Easing.OutQuint);
@@ -204,7 +215,7 @@ private void tryUpdateSliderFromTextBox()
 
             try
             {
-                switch (Current)
+                switch (currentNumberInstantaneous)
                 {
                     case Bindable<int> bindableInt:
                         bindableInt.Value = int.Parse(textBox.Current.Value);
@@ -215,7 +226,7 @@ private void tryUpdateSliderFromTextBox()
                         break;
 
                     default:
-                        Current.Parse(textBox.Current.Value, CultureInfo.CurrentCulture);
+                        currentNumberInstantaneous.Parse(textBox.Current.Value, CultureInfo.CurrentCulture);
                         break;
                 }
             }
@@ -250,9 +261,9 @@ private void updateState()
         {
             textBox.Alpha = 1;
 
-            background.Colour = Current.Disabled ? colourProvider.Background4 : colourProvider.Background5;
-            caption.Colour = Current.Disabled ? colourProvider.Foreground1 : colourProvider.Content2;
-            textBox.Colour = Current.Disabled ? colourProvider.Foreground1 : colourProvider.Content1;
+            background.Colour = currentNumberInstantaneous.Disabled ? colourProvider.Background4 : colourProvider.Background5;
+            caption.Colour = currentNumberInstantaneous.Disabled ? colourProvider.Foreground1 : colourProvider.Content2;
+            textBox.Colour = currentNumberInstantaneous.Disabled ? colourProvider.Foreground1 : colourProvider.Content1;
 
             BorderThickness = IsHovered || textBox.Focused.Value || slider.IsDragging.Value ? 2 : 0;
             BorderColour = textBox.Focused.Value ? colourProvider.Highlight1 : colourProvider.Light4;
@@ -269,12 +280,13 @@ private void updateValueDisplay()
         {
             if (updatingFromTextBox) return;
 
-            textBox.Text = slider.GetDisplayableValue(Current.Value).ToString();
+            textBox.Text = slider.GetDisplayableValue(currentNumberInstantaneous.Value).ToString();
         }
 
         private partial class Slider : OsuSliderBar<T>
         {
             public BindableBool IsDragging { get; set; } = new BindableBool();
+            public Action? OnCommit { get; set; }
 
             private Box leftBox = null!;
             private Box rightBox = null!;
@@ -381,6 +393,16 @@ protected override void UpdateValue(float value)
             {
                 nub.MoveToX(value, 200, Easing.OutPow10);
             }
+
+            protected override bool Commit()
+            {
+                bool result = base.Commit();
+
+                if (result)
+                    OnCommit?.Invoke();
+
+                return result;
+            }
         }
     }
 }