From 5096f6b83dca93e86ee4288f5f61cefcd63f6f04 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Feb 2017 20:59:21 +0900 Subject: [PATCH 1/7] Fix memory leak from Player's InputManager. --- osu.Game/Screens/Play/Player.cs | 9 +++++++++ osu.Game/Screens/Play/PlayerLoader.cs | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index b106ea60c6..d45216a5f7 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -222,6 +222,12 @@ namespace osu.Game.Screens.Play if (IsPaused) Pause(); else Resume(); } + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + playerInputManager?.Dispose(); + } + public void Restart() { sourceClock.Stop(); // If the clock is running and Restart is called the game will lag until relaunch @@ -312,6 +318,9 @@ namespace osu.Game.Screens.Play } else { + FadeOut(250); + Content.ScaleTo(0.7f, 750, EasingTypes.InQuint); + dimLevel.ValueChanged -= dimChanged; Background?.FadeTo(1f, 200); return base.OnExiting(next); diff --git a/osu.Game/Screens/Play/PlayerLoader.cs b/osu.Game/Screens/Play/PlayerLoader.cs index f14ae65f68..ba7c392b84 100644 --- a/osu.Game/Screens/Play/PlayerLoader.cs +++ b/osu.Game/Screens/Play/PlayerLoader.cs @@ -89,6 +89,12 @@ namespace osu.Game.Screens.Play { Content.ScaleTo(0.7f, 150, EasingTypes.InQuint); FadeOut(150); + + //this is required to clean up the InputManager in Player. + //can be removed once we solve that one. + if (player != null && player.LoadState != LoadState.Alive) + player.Dispose(); + return base.OnExiting(next); } From c4fbfb5a8f55a519e0b872904f26fbcc63e30e0d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Feb 2017 21:46:34 +0900 Subject: [PATCH 2/7] Update comment with new knowledge. --- osu.Game/Screens/Play/PlayerLoader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/PlayerLoader.cs b/osu.Game/Screens/Play/PlayerLoader.cs index ba7c392b84..4515eb75a9 100644 --- a/osu.Game/Screens/Play/PlayerLoader.cs +++ b/osu.Game/Screens/Play/PlayerLoader.cs @@ -90,7 +90,7 @@ namespace osu.Game.Screens.Play Content.ScaleTo(0.7f, 150, EasingTypes.InQuint); FadeOut(150); - //this is required to clean up the InputManager in Player. + //OsuScreens are currently never finalised due to the Bindable bindings. //can be removed once we solve that one. if (player != null && player.LoadState != LoadState.Alive) player.Dispose(); From 7483b6947769f070e9d532e8f085c9c7833d4612 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 25 Feb 2017 10:44:19 +0900 Subject: [PATCH 3/7] Remove unnecessary Dispose logic. --- osu.Game/Screens/Play/Player.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index d45216a5f7..35154b2f45 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -222,12 +222,6 @@ namespace osu.Game.Screens.Play if (IsPaused) Pause(); else Resume(); } - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - playerInputManager?.Dispose(); - } - public void Restart() { sourceClock.Stop(); // If the clock is running and Restart is called the game will lag until relaunch From fdf5867a1a6beb07bb5446651a01fb73337f9c85 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 25 Feb 2017 11:46:04 +0900 Subject: [PATCH 4/7] Add more informative asserts in unit tests. --- osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index fe0abd6d87..24d2ad9209 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -84,16 +84,16 @@ namespace osu.Game.Tests.Beatmaps.IO Action waitAction = () => { while ((resultSets = osu.Dependencies.Get() - .Query().Where(s => s.OnlineBeatmapSetID == 241526)).Count() != 1) - Thread.Sleep(1); + .Query().Where(s => s.OnlineBeatmapSetID == 241526)).Count() == 0) + Thread.Sleep(50); }; Assert.IsTrue(waitAction.BeginInvoke(null, null).AsyncWaitHandle.WaitOne(timeout), - @"BeatmapSet did not import to the database"); + $@"BeatmapSet did not import to the database in allocated time."); //ensure we were stored to beatmap database backing... - Assert.IsTrue(resultSets.Count() == 1); + Assert.IsTrue(resultSets.Count() == 1, $@"Incorrect result count found ({resultSets.Count()} but should be 1)."); IEnumerable resultBeatmaps = null; From 17a28cd3b67bad81327db41cdbcd44c1eb5f3bfa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 25 Feb 2017 11:46:19 +0900 Subject: [PATCH 5/7] Don't push Player if we aren't still current. --- osu.Game/Screens/Play/PlayerLoader.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Play/PlayerLoader.cs b/osu.Game/Screens/Play/PlayerLoader.cs index 4515eb75a9..87d6a82404 100644 --- a/osu.Game/Screens/Play/PlayerLoader.cs +++ b/osu.Game/Screens/Play/PlayerLoader.cs @@ -80,6 +80,8 @@ namespace osu.Game.Screens.Play Schedule(() => { + if (!IsCurrentScreen) return; + if (!Push(player)) Exit(); }); From 9067907789810c9628b766bd5fdac0985c0be0e6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 25 Feb 2017 11:55:30 +0900 Subject: [PATCH 6/7] Better assertions. --- osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 24d2ad9209..cb4a4bd5f2 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -102,16 +102,17 @@ namespace osu.Game.Tests.Beatmaps.IO { while ((resultBeatmaps = osu.Dependencies.Get() .Query().Where(s => s.OnlineBeatmapSetID == 241526 && s.BaseDifficultyID > 0)).Count() != 12) - Thread.Sleep(1); + Thread.Sleep(50); }; Assert.IsTrue(waitAction.BeginInvoke(null, null).AsyncWaitHandle.WaitOne(timeout), - @"Beatmaps did not import to the database"); + @"Beatmaps did not import to the database in allocated time"); //fetch children and check we can load from the post-storage path... var set = osu.Dependencies.Get().GetChildren(resultSets.First()); - Assert.IsTrue(set.Beatmaps.Count == resultBeatmaps.Count()); + Assert.IsTrue(set.Beatmaps.Count == resultBeatmaps.Count(), + $@"Incorrect database beatmap count post-import ({resultBeatmaps.Count()} but should be {set.Beatmaps.Count})."); foreach (BeatmapInfo b in resultBeatmaps) Assert.IsTrue(set.Beatmaps.Any(c => c.OnlineBeatmapID == b.OnlineBeatmapID)); From 6a67ffa5b3e5ffb9be3c5a790dc184aec1e67bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Sat, 25 Feb 2017 08:31:29 +0100 Subject: [PATCH 7/7] Attempt to fix failing beatmap import test case --- osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index cb4a4bd5f2..8de1a0f918 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -59,7 +59,7 @@ namespace osu.Game.Tests.Beatmaps.IO if (!importer.ImportAsync(osz_path).Wait(1000)) Assert.Fail(@"IPC took too long to send"); - ensureLoaded(osu, 10000); + ensureLoaded(osu); } } @@ -77,7 +77,7 @@ namespace osu.Game.Tests.Beatmaps.IO return osu; } - private void ensureLoaded(OsuGameBase osu, int timeout = 100) + private void ensureLoaded(OsuGameBase osu, int timeout = 10000) { IEnumerable resultSets = null;