From cc356bcfe4afacb6392e22bdfa665cf4f3249cc9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 15 Mar 2022 18:57:53 +0900 Subject: [PATCH 1/7] Show components available for current screen only (using actual live dependencies) --- .../Skinning/Editor/SkinComponentToolbox.cs | 76 ++++++++----------- osu.Game/Skinning/Editor/SkinEditor.cs | 31 ++++---- 2 files changed, 47 insertions(+), 60 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinComponentToolbox.cs b/osu.Game/Skinning/Editor/SkinComponentToolbox.cs index 483e365e78..9c4a369c1d 100644 --- a/osu.Game/Skinning/Editor/SkinComponentToolbox.cs +++ b/osu.Game/Skinning/Editor/SkinComponentToolbox.cs @@ -2,24 +2,16 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Generic; using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input.Events; -using osu.Framework.Utils; -using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; -using osu.Game.Rulesets; -using osu.Game.Rulesets.Difficulty; -using osu.Game.Rulesets.Mods; -using osu.Game.Rulesets.Scoring; -using osu.Game.Rulesets.UI; using osu.Game.Screens.Edit.Components; using osuTK; @@ -29,26 +21,19 @@ namespace osu.Game.Skinning.Editor { public Action RequestPlacement; - [Cached] - private ScoreProcessor scoreProcessor = new ScoreProcessor(new DummyRuleset()) - { - Combo = { Value = RNG.Next(1, 1000) }, - TotalScore = { Value = RNG.Next(1000, 10000000) } - }; + private readonly CompositeDrawable target; - [Cached(typeof(HealthProcessor))] - private HealthProcessor healthProcessor = new DrainingHealthProcessor(0); - - public SkinComponentToolbox() + public SkinComponentToolbox(CompositeDrawable target = null) : base("Components") { + this.target = target; } + private FillFlowContainer fill; + [BackgroundDependencyLoader] private void load() { - FillFlowContainer fill; - Child = fill = new FillFlowContainer { RelativeSizeAxes = Axes.X, @@ -57,6 +42,13 @@ namespace osu.Game.Skinning.Editor Spacing = new Vector2(2) }; + reloadComponents(); + } + + private void reloadComponents() + { + fill.Clear(); + var skinnableTypes = typeof(OsuGame).Assembly.GetTypes() .Where(t => !t.IsInterface) .Where(t => typeof(ISkinnableDrawable).IsAssignableFrom(t)) @@ -64,18 +56,10 @@ namespace osu.Game.Skinning.Editor .ToArray(); foreach (var type in skinnableTypes) - { - var component = attemptAddComponent(type); - - if (component != null) - { - component.RequestPlacement = t => RequestPlacement?.Invoke(t); - fill.Add(component); - } - } + attemptAddComponent(type); } - private static ToolboxComponentButton attemptAddComponent(Type type) + private void attemptAddComponent(Type type) { try { @@ -83,14 +67,15 @@ namespace osu.Game.Skinning.Editor Debug.Assert(instance != null); - if (!((ISkinnableDrawable)instance).IsEditable) - return null; + if (!((ISkinnableDrawable)instance).IsEditable) return; - return new ToolboxComponentButton(instance); + fill.Add(new ToolboxComponentButton(instance, target) + { + RequestPlacement = t => RequestPlacement?.Invoke(t) + }); } catch { - return null; } } @@ -101,6 +86,7 @@ namespace osu.Game.Skinning.Editor public override bool PropagateNonPositionalInputSubTree => false; private readonly Drawable component; + private readonly CompositeDrawable dependencySource; public Action RequestPlacement; @@ -109,9 +95,10 @@ namespace osu.Game.Skinning.Editor private const float contracted_size = 60; private const float expanded_size = 120; - public ToolboxComponentButton(Drawable component) + public ToolboxComponentButton(Drawable component, CompositeDrawable dependencySource) { this.component = component; + this.dependencySource = dependencySource; Enabled.Value = true; @@ -143,7 +130,7 @@ namespace osu.Game.Skinning.Editor RelativeSizeAxes = Axes.Both, Padding = new MarginPadding(10) { Bottom = 20 }, Masking = true, - Child = innerContainer = new Container + Child = innerContainer = new DependencyBorrowingContainer(dependencySource) { RelativeSizeAxes = Axes.Both, Anchor = Anchor.Centre, @@ -186,14 +173,17 @@ namespace osu.Game.Skinning.Editor } } - private class DummyRuleset : Ruleset + public class DependencyBorrowingContainer : Container { - public override IEnumerable GetModsFor(ModType type) => throw new NotImplementedException(); - public override DrawableRuleset CreateDrawableRulesetWith(IBeatmap beatmap, IReadOnlyList mods = null) => throw new NotImplementedException(); - public override IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap) => throw new NotImplementedException(); - public override DifficultyCalculator CreateDifficultyCalculator(IWorkingBeatmap beatmap) => throw new NotImplementedException(); - public override string Description => string.Empty; - public override string ShortName => string.Empty; + private readonly CompositeDrawable donor; + + public DependencyBorrowingContainer(CompositeDrawable donor) + { + this.donor = donor; + } + + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) => + new DependencyContainer(donor?.Dependencies ?? base.CreateChildDependencies(parent)); } } } diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index f7e5aeecdf..2a7e3439c0 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -51,6 +51,7 @@ namespace osu.Game.Skinning.Editor private Container content; private EditorSidebarSection settingsToolbox; + private EditorSidebar componentsSidebar; public SkinEditor() { @@ -146,16 +147,7 @@ namespace osu.Game.Skinning.Editor { new Drawable[] { - new EditorSidebar - { - Children = new[] - { - new SkinComponentToolbox - { - RequestPlacement = placeComponent - }, - } - }, + componentsSidebar = new EditorSidebar(), content = new Container { Depth = float.MaxValue, @@ -211,7 +203,15 @@ namespace osu.Game.Skinning.Editor Scheduler.AddOnce(loadBlueprintContainer); Scheduler.AddOnce(populateSettings); - void loadBlueprintContainer() => content.Child = new SkinBlueprintContainer(targetScreen); + void loadBlueprintContainer() + { + content.Child = new SkinBlueprintContainer(targetScreen); + + componentsSidebar.Child = new SkinComponentToolbox(getFirstTarget() as CompositeDrawable) + { + RequestPlacement = placeComponent + }; + } } private void skinChanged() @@ -238,12 +238,7 @@ namespace osu.Game.Skinning.Editor private void placeComponent(Type type) { - var target = availableTargets.FirstOrDefault()?.Target; - - if (target == null) - return; - - var targetContainer = getTarget(target.Value); + var targetContainer = getFirstTarget(); if (targetContainer == null) return; @@ -278,6 +273,8 @@ namespace osu.Game.Skinning.Editor private IEnumerable availableTargets => targetScreen.ChildrenOfType(); + private ISkinnableTarget getFirstTarget() => availableTargets.FirstOrDefault(); + private ISkinnableTarget getTarget(SkinnableTarget target) { return availableTargets.FirstOrDefault(c => c.Target == target); From ac739c9dae7a6ab6ca09eac1e9a9b93eeeac5ce4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 15 Mar 2022 22:55:47 +0900 Subject: [PATCH 2/7] Change `PerformancePointsCounter` resolution requirements to be required All other similar UI components have required dependencies, so this is mainly to bring things in line with expectations. I am using this fact in the skin editor to only show components which can be used in the current editor context (by `try-catch`ing their `Activator.CreateInstance`). --- .../Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs | 5 +++++ osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs | 4 ++++ .../Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs | 4 ++++ .../Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs | 4 ++++ osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs | 6 ++---- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index c5ea9e6204..c70313ee8a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -18,9 +18,11 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Skinning.Legacy; using osu.Game.Rulesets.Scoring; +using osu.Game.Screens.Play; using osu.Game.Screens.Play.HUD; using osu.Game.Skinning; using osu.Game.Storyboards; +using osu.Game.Tests.Beatmaps; namespace osu.Game.Tests.Visual.Gameplay { @@ -37,6 +39,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(HealthProcessor))] private HealthProcessor healthProcessor = new DrainingHealthProcessor(0); + [Cached] + private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + protected override bool HasCustomSteps => true; [Test] diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs index 505f73159f..d6a4d9bf79 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs @@ -14,6 +14,7 @@ using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play; using osu.Game.Skinning; +using osu.Game.Tests.Beatmaps; using osuTK.Input; namespace osu.Game.Tests.Visual.Gameplay @@ -30,6 +31,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(HealthProcessor))] private HealthProcessor healthProcessor = new DrainingHealthProcessor(0); + [Cached] + private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + // best way to check without exposing. private Drawable hideTarget => hudOverlay.KeyCounter; private FillFlowContainer keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType>().First(); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs index 8f33f6fac5..5dc4bdb041 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs @@ -10,6 +10,7 @@ using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play; using osu.Game.Skinning.Editor; +using osu.Game.Tests.Beatmaps; using osuTK.Input; namespace osu.Game.Tests.Visual.Gameplay @@ -22,6 +23,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(HealthProcessor))] private HealthProcessor healthProcessor = new DrainingHealthProcessor(0); + [Cached] + private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + [SetUpSteps] public void SetUpSteps() { diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs index cdf349ff7f..da443a6c92 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs @@ -15,6 +15,7 @@ using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play; +using osu.Game.Tests.Beatmaps; using osuTK.Input; namespace osu.Game.Tests.Visual.Gameplay @@ -29,6 +30,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(HealthProcessor))] private HealthProcessor healthProcessor = new DrainingHealthProcessor(0); + [Cached] + private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + private IEnumerable hudOverlays => CreatedDrawables.OfType(); // best way to check without exposing. diff --git a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs index c0d0ea0721..7a1f724cfb 100644 --- a/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs +++ b/osu.Game/Screens/Play/HUD/PerformancePointsCounter.cs @@ -42,12 +42,10 @@ namespace osu.Game.Screens.Play.HUD private const float alpha_when_invalid = 0.3f; - [CanBeNull] - [Resolved(CanBeNull = true)] + [Resolved] private ScoreProcessor scoreProcessor { get; set; } - [Resolved(CanBeNull = true)] - [CanBeNull] + [Resolved] private GameplayState gameplayState { get; set; } [CanBeNull] From fd71aa4a4d321078f0da1194db47f18590052c8f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 17 Mar 2022 15:05:51 +0900 Subject: [PATCH 3/7] Change `SongProgress` resolution requirements to be required --- osu.Game/Screens/Play/SongProgress.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/SongProgress.cs b/osu.Game/Screens/Play/SongProgress.cs index b27a9c5f5d..694b5ec628 100644 --- a/osu.Game/Screens/Play/SongProgress.cs +++ b/osu.Game/Screens/Play/SongProgress.cs @@ -73,9 +73,12 @@ namespace osu.Game.Screens.Play [Resolved(canBeNull: true)] private Player player { get; set; } - [Resolved(canBeNull: true)] + [Resolved] private GameplayClock gameplayClock { get; set; } + [Resolved] + private DrawableRuleset drawableRuleset { get; set; } + private IClock referenceClock; public bool UsesFixedAnchor { get; set; } @@ -113,7 +116,7 @@ namespace osu.Game.Screens.Play } [BackgroundDependencyLoader(true)] - private void load(OsuColour colours, OsuConfigManager config, DrawableRuleset drawableRuleset) + private void load(OsuColour colours, OsuConfigManager config) { base.LoadComplete(); From e252f1a3cd109e0cc758ff8b30af9b3a2541adbb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 17 Mar 2022 18:40:26 +0900 Subject: [PATCH 4/7] Add explanation about try-catch logic in `SkinComponentToolbox` --- osu.Game/Skinning/Editor/SkinComponentToolbox.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/Editor/SkinComponentToolbox.cs b/osu.Game/Skinning/Editor/SkinComponentToolbox.cs index 9c4a369c1d..a586cdcd54 100644 --- a/osu.Game/Skinning/Editor/SkinComponentToolbox.cs +++ b/osu.Game/Skinning/Editor/SkinComponentToolbox.cs @@ -8,6 +8,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input.Events; +using osu.Framework.Logging; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; @@ -74,8 +75,14 @@ namespace osu.Game.Skinning.Editor RequestPlacement = t => RequestPlacement?.Invoke(t) }); } - catch + catch (DependencyNotRegisteredException) { + // This loading code relies on try-catching any dependency injection errors to know which components are valid for the current target screen. + // If a screen can't provide the required dependencies, a skinnable component should not be displayed in the list. + } + catch (Exception e) + { + Logger.Error(e, $"Skin component {type} could not be loaded in the editor component list due to an error"); } } From 4650c197a30dc311a072dc292f4fb6a5f5929ba4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 17 Mar 2022 18:45:35 +0900 Subject: [PATCH 5/7] Make `SongProgress.DrawableRuleset` nullable to allow test scene to work --- osu.Game/Screens/Play/SongProgress.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/SongProgress.cs b/osu.Game/Screens/Play/SongProgress.cs index 694b5ec628..e620abb90f 100644 --- a/osu.Game/Screens/Play/SongProgress.cs +++ b/osu.Game/Screens/Play/SongProgress.cs @@ -76,7 +76,7 @@ namespace osu.Game.Screens.Play [Resolved] private GameplayClock gameplayClock { get; set; } - [Resolved] + [Resolved(canBeNull: true)] private DrawableRuleset drawableRuleset { get; set; } private IClock referenceClock; From ef29de997135db93b9fe1405a2ba14d34857ed17 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 17 Mar 2022 19:50:43 +0900 Subject: [PATCH 6/7] Add one more missing dependency required by `SongProgress` --- .../Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index c70313ee8a..eb1695b3df 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -42,6 +42,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached] private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + [Cached] + private readonly GameplayClock gameplayClock = new GameplayClock(new FramedClock()); + protected override bool HasCustomSteps => true; [Test] From a8bb696e5b28e5f87c663f7315c9c819608e7ab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 17 Mar 2022 21:22:07 +0100 Subject: [PATCH 7/7] Cache gameplay clock in more places to fix song progress related tests --- osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs | 4 ++++ .../Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs | 4 ++++ .../Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs index d6a4d9bf79..2d12645811 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs @@ -8,6 +8,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; +using osu.Framework.Timing; using osu.Game.Configuration; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; @@ -34,6 +35,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached] private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + [Cached] + private readonly GameplayClock gameplayClock = new GameplayClock(new FramedClock()); + // best way to check without exposing. private Drawable hideTarget => hudOverlay.KeyCounter; private FillFlowContainer keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType>().First(); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs index 5dc4bdb041..8150252d45 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs @@ -5,6 +5,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; +using osu.Framework.Timing; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; @@ -26,6 +27,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached] private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + [Cached] + private readonly GameplayClock gameplayClock = new GameplayClock(new FramedClock()); + [SetUpSteps] public void SetUpSteps() { diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs index da443a6c92..ac5e408d90 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs @@ -10,6 +10,7 @@ using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; +using osu.Framework.Timing; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; @@ -33,6 +34,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached] private GameplayState gameplayState = new GameplayState(new TestBeatmap(new OsuRuleset().RulesetInfo), new OsuRuleset()); + [Cached] + private readonly GameplayClock gameplayClock = new GameplayClock(new FramedClock()); + private IEnumerable hudOverlays => CreatedDrawables.OfType(); // best way to check without exposing.