Merge pull request #2791 from peppy/fix-memory-leak

Fix gameplay memory leak due to incorrect binding

COMPILING.md

@@ -1,36 +0,0 @@
-# Linux
-### 1. Requirements:
-Mono >= 5.4.0 (>= 5.8.0 recommended)
-Please check [here](http://www.mono-project.com/download/) for stable or [here](http://www.mono-project.com/download/alpha/) for an alpha release.  
-NuGet >= 4.4.0  
-msbuild  
-git
-
-### 2. Cloning project
-Clone the entire repository with submodules using
-```
-git clone https://github.com/ppy/osu --recursive
-```
-Then restore NuGet packages from the repository
-```
-nuget restore
-```
-### 3. Compiling
-Simply run `msbuild` where `osu.sln` is located, this will create all binaries in `osu/osu.Desktop/bin/Debug`.
-### 4. Optimizing
-If you want additional performance you can change build type to Release with
-```
-msbuild -p:Configuration=Release
-```
-Additionally, mono provides an AOT utility which attempts to precompile binaries. You can utilize that by running
-```
-mono --aot ./osu\!.exe
-```
-### 5. Troubleshooting
-You may run into trouble with NuGet versioning, as the one in packaging system is almost always out of date. Simply run 
-```
-nuget
-sudo nuget update -self
-```
-**Warning** NuGet creates few config files when it's run for the first time.
-Do not run NuGet as root on the first run or you might run into very peculiar issues.

osu.Game.Rulesets.Osu/UI/Cursor/GameplayCursor.cs

@@ -161,7 +161,7 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor
                 };
 
                 this.beatmap.BindTo(beatmap);
-                beatmap.ValueChanged += v => calculateScale();
+                this.beatmap.ValueChanged += v => calculateScale();
 
                 cursorScale = config.GetBindable<double>(OsuSetting.GameplayCursorSize);
                 cursorScale.ValueChanged += v => calculateScale();

osu.Game.Rulesets.Taiko/TaikoRuleset.cs

@@ -91,7 +91,7 @@ namespace osu.Game.Rulesets.Taiko
                     {
                         new TaikoModHardRock(),
                         new MultiMod(new TaikoModSuddenDeath(), new TaikoModPerfect()),
-                        new MultiMod(new TaikoModDoubleTime(), new TaikoModDaycore()),
+                        new MultiMod(new TaikoModDoubleTime(), new TaikoModNightcore()),
                         new TaikoModHidden(),
                         new TaikoModFlashlight(),
                     };

osu.Game/Rulesets/UI/Scrolling/ScrollingPlayfield.cs

@@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.UI.Scrolling
         /// <summary>
         /// The step increase/decrease of the span of time visible by the length of the scrolling axes.
         /// </summary>
-        private const double time_span_step = 50;
+        private const double time_span_step = 200;
 
         /// <summary>
         /// The span of time that is visible by the length of the scrolling axes.
@@ -88,10 +88,10 @@ namespace osu.Game.Rulesets.UI.Scrolling
                 switch (args.Key)
                 {
                     case Key.Minus:
-                        this.TransformBindableTo(VisibleTimeRange, VisibleTimeRange + time_span_step, 200, Easing.OutQuint);
+                        this.TransformBindableTo(VisibleTimeRange, VisibleTimeRange + time_span_step, 600, Easing.OutQuint);
                         break;
                     case Key.Plus:
-                        this.TransformBindableTo(VisibleTimeRange, VisibleTimeRange - time_span_step, 200, Easing.OutQuint);
+                        this.TransformBindableTo(VisibleTimeRange, VisibleTimeRange - time_span_step, 600, Easing.OutQuint);
                         break;
                 }
             }

osu.Game/Screens/Play/KeyCounterCollection.cs

@@ -18,7 +18,8 @@ namespace osu.Game.Screens.Play
     {
         private const int duration = 100;
 
-        private Bindable<bool> showKeyCounter;
+        public readonly Bindable<bool> Visible = new Bindable<bool>(true);
+        private readonly Bindable<bool> configVisibility = new Bindable<bool>();
 
         public KeyCounterCollection()
         {
@@ -46,9 +47,10 @@ namespace osu.Game.Screens.Play
         [BackgroundDependencyLoader]
         private void load(OsuConfigManager config)
         {
-            showKeyCounter = config.GetBindable<bool>(OsuSetting.KeyOverlay);
-            showKeyCounter.ValueChanged += keyCounterVisibility => this.FadeTo(keyCounterVisibility ? 1 : 0, duration);
-            showKeyCounter.TriggerChange();
+            config.BindWith(OsuSetting.KeyOverlay, configVisibility);
+
+            Visible.BindValueChanged(_ => updateVisibility());
+            configVisibility.BindValueChanged(_ => updateVisibility(), true);
         }
 
         //further: change default values here and in KeyCounter if needed, instead of passing them in every constructor
@@ -111,6 +113,8 @@ namespace osu.Game.Screens.Play
             }
         }
 
+        private void updateVisibility() => this.FadeTo(Visible.Value || configVisibility.Value ? 1 : 0, duration);
+
         public override bool HandleKeyboardInput => receptor == null;
         public override bool HandleMouseInput => receptor == null;
 

osu.Game/Screens/Play/Player.cs

@@ -229,6 +229,7 @@ namespace osu.Game.Screens.Play
             };
 
             hudOverlay.HoldToQuit.Action = Exit;
+            hudOverlay.KeyCounter.Visible.BindTo(RulesetContainer.HasReplayLoaded);
 
             if (ShowStoryboard)
                 initializeStoryboard(false);

osu.Game/Tests/Visual/TestCasePlayer.cs

@@ -1,9 +1,11 @@
 // Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
+using System;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics.Shapes;
+using osu.Framework.Lists;
 using osu.Game.Beatmaps;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
@@ -51,6 +53,28 @@ namespace osu.Game.Tests.Visual
                     Player p = null;
                     AddStep(r.Name, () => p = loadPlayerFor(r));
                     AddUntilStep(() => ContinueCondition(p));
+
+                    AddAssert("no leaked beatmaps", () =>
+                    {
+                        p = null;
+
+                        GC.Collect();
+                        GC.WaitForPendingFinalizers();
+                        int count = 0;
+
+                        workingWeakReferences.ForEachAlive(_ => count++);
+                        return count == 1;
+                    });
+
+                    AddAssert("no leaked players", () =>
+                    {
+                        GC.Collect();
+                        GC.WaitForPendingFinalizers();
+                        int count = 0;
+
+                        playerWeakReferences.ForEachAlive(_ => count++);
+                        return count == 1;
+                    });
                 }
             }
         }
@@ -59,21 +83,32 @@ namespace osu.Game.Tests.Visual
 
         protected virtual IBeatmap CreateBeatmap(Ruleset ruleset) => new TestBeatmap(ruleset.RulesetInfo);
 
+        private readonly WeakList<WorkingBeatmap> workingWeakReferences = new WeakList<WorkingBeatmap>();
+        private readonly WeakList<Player> playerWeakReferences = new WeakList<Player>();
+
         private Player loadPlayerFor(RulesetInfo ri) => loadPlayerFor(ri.CreateInstance());
 
         private Player loadPlayerFor(Ruleset r)
         {
             var beatmap = CreateBeatmap(r);
+            var working = new TestWorkingBeatmap(beatmap);
 
-            Beatmap.Value = new TestWorkingBeatmap(beatmap);
+            workingWeakReferences.Add(working);
+
+            Beatmap.Value = working;
             Beatmap.Value.Mods.Value = new[] { r.GetAllMods().First(m => m is ModNoFail) };
 
-            if (Player != null)
-                Remove(Player);
+            Player?.Exit();
 
             var player = CreatePlayer(r);
 
-            LoadComponentAsync(player, LoadScreen);
+            playerWeakReferences.Add(player);
+
+            LoadComponentAsync(player, p =>
+            {
+                Player = p;
+                LoadScreen(p);
+            });
 
             return player;
         }