Merge pull request #16488 from peppy/global-bindable-thread-safety

Ensure global beatmap/ruleset are always mutated from the update thread

osu.Game.Tests/Visual/Editing/TestSceneComposeScreen.cs

@@ -29,9 +29,10 @@ namespace osu.Game.Tests.Visual.Editing
         [Cached]
         private EditorClipboard clipboard = new EditorClipboard();
 
-        [BackgroundDependencyLoader]
+        protected override void LoadComplete()
-        private void load()
         {
+            base.LoadComplete();
+
             Beatmap.Value = CreateWorkingBeatmap(editorBeatmap.PlayableBeatmap);
 
             Child = new ComposeScreen

osu.Game.Tests/Visual/Editing/TestSceneEditorClock.cs

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using NUnit.Framework;
-using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Rulesets.Osu;
@@ -37,9 +36,10 @@ namespace osu.Game.Tests.Visual.Editing
             });
         }
 
-        [BackgroundDependencyLoader]
+        protected override void LoadComplete()
-        private void load()
         {
+            base.LoadComplete();
+
             Beatmap.Value = CreateWorkingBeatmap(new OsuRuleset().RulesetInfo);
             // ensure that music controller does not change this beatmap due to it
             // completing naturally as part of the test.

osu.Game.Tests/Visual/Editing/TestSceneEditorSeekSnapping.cs

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using NUnit.Framework;
-using osu.Framework.Allocation;
 using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
@@ -23,9 +22,10 @@ namespace osu.Game.Tests.Visual.Editing
             BeatDivisor.Value = 4;
         }
 
-        [BackgroundDependencyLoader]
+        protected override void LoadComplete()
-        private void load()
         {
+            base.LoadComplete();
+
             var testBeatmap = new Beatmap
             {
                 ControlPointInfo = new ControlPointInfo(),

osu.Game.Tests/Visual/Editing/TestSceneTimingScreen.cs

@@ -29,9 +29,10 @@ namespace osu.Game.Tests.Visual.Editing
             editorBeatmap = new EditorBeatmap(CreateBeatmap(new OsuRuleset().RulesetInfo));
         }
 
-        [BackgroundDependencyLoader]
+        protected override void LoadComplete()
-        private void load()
         {
+            base.LoadComplete();
+
             Beatmap.Value = CreateWorkingBeatmap(editorBeatmap.PlayableBeatmap);
             Beatmap.Disabled = true;
 

osu.Game.Tests/Visual/Navigation/TestSceneStartupRuleset.cs

@@ -0,0 +1,32 @@
+// 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.Development;
+using osu.Game.Configuration;
+
+namespace osu.Game.Tests.Visual.Navigation
+{
+    [TestFixture]
+    public class TestSceneStartupRuleset : OsuGameTestScene
+    {
+        protected override TestOsuGame CreateTestGame()
+        {
+            // Must be done in this function due to the RecycleLocalStorage call just before.
+            var config = DebugUtils.IsDebugBuild
+                ? new DevelopmentOsuConfigManager(LocalStorage)
+                : new OsuConfigManager(LocalStorage);
+
+            config.SetValue(OsuSetting.Ruleset, "mania");
+            config.Save();
+
+            return base.CreateTestGame();
+        }
+
+        [Test]
+        public void TestRulesetConsumed()
+        {
+            AddUntilStep("ruleset correct", () => Game.Ruleset.Value.ShortName == "mania");
+        }
+    }
+}

osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs

@@ -35,9 +35,9 @@ namespace osu.Game.Tournament.Tests.NonVisual
 
         public class TestTournament : TournamentGameBase
         {
-            [BackgroundDependencyLoader]
+            protected override void LoadComplete()
-            private void load()
             {
+                base.LoadComplete();
                 Ruleset.Value = new RulesetInfo(); // not available
             }
         }

osu.Game/OsuGameBase.cs

@@ -300,6 +300,7 @@ namespace osu.Game
             dependencies.CacheAs(MusicController);
 
             Ruleset.BindValueChanged(onRulesetChanged);
+            Beatmap.BindValueChanged(onBeatmapChanged);
         }
 
         protected virtual void InitialiseFonts()
@@ -420,8 +421,17 @@ namespace osu.Game
 
         protected override Storage CreateStorage(GameHost host, Storage defaultStorage) => new OsuStorage(host, defaultStorage);
 
+        private void onBeatmapChanged(ValueChangedEvent<WorkingBeatmap> valueChangedEvent)
+        {
+            if (IsLoaded && !ThreadSafety.IsUpdateThread)
+                throw new InvalidOperationException("Global beatmap bindable must be changed from update thread.");
+        }
+
         private void onRulesetChanged(ValueChangedEvent<RulesetInfo> r)
         {
+            if (IsLoaded && !ThreadSafety.IsUpdateThread)
+                throw new InvalidOperationException("Global ruleset bindable must be changed from update thread.");
+
             Ruleset instance = null;
 
             try

osu.Game/Tests/Visual/EditorClockTestScene.cs

@@ -35,9 +35,9 @@ namespace osu.Game.Tests.Visual
             return dependencies;
         }
 
-        [BackgroundDependencyLoader]
+        protected override void LoadComplete()
-        private void load()
         {
+            base.LoadComplete();
             Beatmap.BindValueChanged(beatmapChanged, true);
         }
 

osu.Game/Tests/Visual/EditorTestScene.cs

@@ -42,17 +42,27 @@ namespace osu.Game.Tests.Visual
             Alpha = 0
         };
 
+        private TestBeatmapManager testBeatmapManager;
+        private WorkingBeatmap working;
+
         [BackgroundDependencyLoader]
         private void load(GameHost host, AudioManager audio, RulesetStore rulesets)
         {
             Add(logo);
 
-            var working = CreateWorkingBeatmap(Ruleset.Value);
+            working = CreateWorkingBeatmap(Ruleset.Value);
-
-            Beatmap.Value = working;
 
             if (IsolateSavingFromDatabase)
-                Dependencies.CacheAs<BeatmapManager>(new TestBeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default, working));
+                Dependencies.CacheAs<BeatmapManager>(testBeatmapManager = new TestBeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default));
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            Beatmap.Value = working;
+            if (testBeatmapManager != null)
+                testBeatmapManager.TestBeatmap = working;
         }
 
         protected virtual bool EditorComponentsReady => Editor.ChildrenOfType<HitObjectComposer>().FirstOrDefault()?.IsLoaded == true
@@ -114,12 +124,11 @@ namespace osu.Game.Tests.Visual
 
         private class TestBeatmapManager : BeatmapManager
         {
-            private readonly WorkingBeatmap testBeatmap;
+            public WorkingBeatmap TestBeatmap;
 
-            public TestBeatmapManager(Storage storage, RealmContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, IResourceStore<byte[]> resources, GameHost host, WorkingBeatmap defaultBeatmap, WorkingBeatmap testBeatmap)
+            public TestBeatmapManager(Storage storage, RealmContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, IResourceStore<byte[]> resources, GameHost host, WorkingBeatmap defaultBeatmap)
                 : base(storage, contextFactory, rulesets, api, audioManager, resources, host, defaultBeatmap)
             {
-                this.testBeatmap = testBeatmap;
             }
 
             protected override BeatmapModelManager CreateBeatmapModelManager(Storage storage, RealmContextFactory contextFactory, RulesetStore rulesets, BeatmapOnlineLookupQueue onlineLookupQueue)
@@ -143,7 +152,7 @@ namespace osu.Game.Tests.Visual
                 }
 
                 public override WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo)
-                    => testBeatmapManager.testBeatmap;
+                    => testBeatmapManager.TestBeatmap;
             }
 
             internal class TestBeatmapModelManager : BeatmapModelManager

osu.Game/Tests/Visual/OsuTestScene.cs

@@ -28,7 +28,6 @@ using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.UI;
-using osu.Game.Screens;
 using osu.Game.Storyboards;
 using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Rulesets;
@@ -38,13 +37,16 @@ namespace osu.Game.Tests.Visual
     [ExcludeFromDynamicCompile]
     public abstract class OsuTestScene : TestScene
     {
-        protected Bindable<WorkingBeatmap> Beatmap { get; private set; }
+        [Cached]
+        protected Bindable<WorkingBeatmap> Beatmap { get; } = new Bindable<WorkingBeatmap>();
 
-        protected Bindable<RulesetInfo> Ruleset;
+        [Cached]
+        protected Bindable<RulesetInfo> Ruleset { get; } = new Bindable<RulesetInfo>();
 
-        protected Bindable<IReadOnlyList<Mod>> SelectedMods;
+        [Cached]
+        protected Bindable<IReadOnlyList<Mod>> SelectedMods { get; } = new Bindable<IReadOnlyList<Mod>>(Array.Empty<Mod>());
 
-        protected new OsuScreenDependencies Dependencies { get; private set; }
+        protected new DependencyContainer Dependencies { get; private set; }
 
         protected IResourceStore<byte[]> Resources;
 
@@ -132,17 +134,15 @@ namespace osu.Game.Tests.Visual
 
             var providedRuleset = CreateRuleset();
             if (providedRuleset != null)
-                baseDependencies = rulesetDependencies = new DrawableRulesetDependencies(providedRuleset, baseDependencies);
+                isolatedBaseDependencies = rulesetDependencies = new DrawableRulesetDependencies(providedRuleset, baseDependencies);
 
-            Dependencies = new OsuScreenDependencies(false, baseDependencies);
+            Dependencies = isolatedBaseDependencies;
 
-            Beatmap = Dependencies.Beatmap;
+            Beatmap.Default = parent.Get<Bindable<WorkingBeatmap>>().Default;
             Beatmap.SetDefault();
 
-            Ruleset = Dependencies.Ruleset;
+            Ruleset.Value = CreateRuleset()?.RulesetInfo ?? parent.Get<RulesetStore>().AvailableRulesets.First();
-            Ruleset.SetDefault();
 
-            SelectedMods = Dependencies.Mods;
             SelectedMods.SetDefault();
 
             if (!UseOnlineAPI)
@@ -155,6 +155,23 @@ namespace osu.Game.Tests.Visual
             return Dependencies;
         }
 
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            var parentBeatmap = Parent.Dependencies.Get<Bindable<WorkingBeatmap>>();
+            parentBeatmap.Value = Beatmap.Value;
+            Beatmap.BindTo(parentBeatmap);
+
+            var parentRuleset = Parent.Dependencies.Get<Bindable<RulesetInfo>>();
+            parentRuleset.Value = Ruleset.Value;
+            Ruleset.BindTo(parentRuleset);
+
+            var parentMods = Parent.Dependencies.Get<Bindable<IReadOnlyList<Mod>>>();
+            parentMods.Value = SelectedMods.Value;
+            SelectedMods.BindTo(parentMods);
+        }
+
         protected override Container<Drawable> Content => content ?? base.Content;
 
         private readonly Container content;
@@ -279,12 +296,6 @@ namespace osu.Game.Tests.Visual
         protected virtual WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) =>
             new ClockBackedTestWorkingBeatmap(beatmap, storyboard, Clock, Audio);
 
-        [BackgroundDependencyLoader]
-        private void load(RulesetStore rulesets)
-        {
-            Ruleset.Value = CreateRuleset()?.RulesetInfo ?? rulesets.AvailableRulesets.First();
-        }
-
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);