Completely separate combo colours from DHOs

osu.Game.Rulesets.Catch/Objects/Banana.cs

@@ -2,13 +2,16 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Collections.Generic;
+using osu.Framework.Utils;
 using osu.Game.Audio;
 using osu.Game.Rulesets.Catch.Judgements;
 using osu.Game.Rulesets.Judgements;
+using osu.Game.Rulesets.Objects.Types;
+using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Catch.Objects
 {
-    public class Banana : Fruit
+    public class Banana : Fruit, IHasComboInformation
     {
         /// <summary>
         /// Index of banana in current shower.
@@ -26,6 +29,29 @@ namespace osu.Game.Rulesets.Catch.Objects
             Samples = samples;
         }
 
+        private Color4? colour;
+
+        Color4 IHasComboInformation.GetComboColour(IReadOnlyList<Color4> comboColours)
+        {
+            // override any external colour changes with banananana
+            return colour ??= getBananaColour();
+        }
+
+        private Color4 getBananaColour()
+        {
+            switch (RNG.Next(0, 3))
+            {
+                default:
+                    return new Color4(255, 240, 0, 255);
+
+                case 1:
+                    return new Color4(255, 192, 0, 255);
+
+                case 2:
+                    return new Color4(214, 221, 28, 255);
+            }
+        }
+
         private class BananaHitSampleInfo : HitSampleInfo
         {
             private static string[] lookupNames { get; } = { "metronomelow", "catch-banana" };

osu.Game.Rulesets.Catch/Objects/Drawables/DrawableBanana.cs

@@ -1,10 +1,8 @@
 // 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 System.Collections.Generic;
 using osu.Framework.Graphics;
 using osu.Framework.Utils;
-using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Catch.Objects.Drawables
 {
@@ -15,14 +13,6 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables
         {
         }
 
-        private Color4? colour;
-
-        protected override Color4 GetComboColour(IReadOnlyList<Color4> comboColours)
-        {
-            // override any external colour changes with banananana
-            return colour ??= getBananaColour();
-        }
-
         protected override void UpdateInitialTransforms()
         {
             base.UpdateInitialTransforms();
@@ -46,20 +36,5 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables
             if (Samples != null)
                 Samples.Frequency.Value = 0.77f + ((Banana)HitObject).BananaIndex * 0.006f;
         }
-
-        private Color4 getBananaColour()
-        {
-            switch (RNG.Next(0, 3))
-            {
-                default:
-                    return new Color4(255, 240, 0, 255);
-
-                case 1:
-                    return new Color4(255, 192, 0, 255);
-
-                case 2:
-                    return new Color4(214, 221, 28, 255);
-            }
-        }
     }
 }

osu.Game.Rulesets.Catch/Objects/Drawables/DrawablePalpableCatchHitObject.cs

@@ -1,12 +1,10 @@
 // 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 System.Collections.Generic;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osuTK;
-using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Catch.Objects.Drawables
 {
@@ -40,8 +38,5 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawables
         {
             ScaleContainer.Scale = new Vector2(HitObject.Scale);
         }
-
-        protected override Color4 GetComboColour(IReadOnlyList<Color4> comboColours) =>
-            comboColours[(HitObject.IndexInBeatmap + 1) % comboColours.Count];
     }
 }

osu.Game.Rulesets.Catch/Objects/PalpableCatchHitObject.cs

@@ -1,13 +1,17 @@
 // 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 System.Collections.Generic;
+using osu.Game.Rulesets.Objects.Types;
+using osuTK.Graphics;
+
 namespace osu.Game.Rulesets.Catch.Objects
 {
     /// <summary>
     /// Represents a single object that can be caught by the catcher.
     /// This includes normal fruits, droplets, and bananas but excludes objects that act only as a container of nested hit objects.
     /// </summary>
-    public abstract class PalpableCatchHitObject : CatchHitObject
+    public abstract class PalpableCatchHitObject : CatchHitObject, IHasComboInformation
     {
         /// <summary>
         /// Difference between the distance to the next object
@@ -25,5 +29,7 @@ namespace osu.Game.Rulesets.Catch.Objects
         /// The target fruit if we are to initiate a hyperdash.
         /// </summary>
         public CatchHitObject HyperDashTarget;
+
+        Color4 IHasComboInformation.GetComboColour(IReadOnlyList<Color4> comboColours) => comboColours[(IndexInBeatmap + 1) % comboColours.Count];
     }
 }

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -529,11 +529,10 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
         private void updateComboColour()
         {
-            if (!(HitObject is IHasComboInformation)) return;
+            if (!(HitObject is IHasComboInformation combo)) return;
 
-            var comboColours = CurrentSkin.GetConfig<GlobalSkinColours, IReadOnlyList<Color4>>(GlobalSkinColours.ComboColours)?.Value;
+            var comboColours = CurrentSkin.GetConfig<GlobalSkinColours, IReadOnlyList<Color4>>(GlobalSkinColours.ComboColours)?.Value ?? Array.Empty<Color4>();
-
+            AccentColour.Value = combo.GetComboColour(comboColours);
-            AccentColour.Value = GetComboColour(comboColours);
         }
 
         /// <summary>
@@ -544,6 +543,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// This will only be called if the <see cref="HitObject"/> implements <see cref="IHasComboInformation"/>.
         /// </remarks>
         /// <param name="comboColours">A list of combo colours provided by the beatmap or skin. Can be null if not available.</param>
+        [Obsolete("Unused. Implement IHasComboInformation and IHasComboInformation.GetComboColour() on the HitObject model instead.")] // Can be removed 20210527
         protected virtual Color4 GetComboColour(IReadOnlyList<Color4> comboColours)
         {
             if (!(HitObject is IHasComboInformation combo))

osu.Game/Rulesets/Objects/Types/IHasComboInformation.cs

@@ -1,7 +1,10 @@
 // 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 System.Collections.Generic;
+using JetBrains.Annotations;
 using osu.Framework.Bindables;
+using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Objects.Types
 {
@@ -35,5 +38,13 @@ namespace osu.Game.Rulesets.Objects.Types
         /// Whether this is the last object in the current combo.
         /// </summary>
         bool LastInCombo { get; set; }
+
+        /// <summary>
+        /// Retrieves the colour of the combo described by this <see cref="IHasComboInformation"/> object from a set of possible combo colours.
+        /// Defaults to using <see cref="ComboIndex"/> to decide the colour.
+        /// </summary>
+        /// <param name="comboColours">A list of possible combo colours provided by the beatmap or skin.</param>
+        /// <returns>The colour of the combo described by this <see cref="IHasComboInformation"/> object.</returns>
+        Color4 GetComboColour([NotNull] IReadOnlyList<Color4> comboColours) => comboColours.Count > 0 ? comboColours[ComboIndex % comboColours.Count] : Color4.White;
     }
 }

osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -19,8 +18,8 @@ using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Objects;
-using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Objects.Types;
+using osu.Game.Skinning;
 using osuTK;
 using osuTK.Graphics;
 
@@ -28,35 +27,26 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
 {
     public class TimelineHitObjectBlueprint : SelectionBlueprint
     {
-        private readonly Circle circle;
+        private const float thickness = 5;
+        private const float shadow_radius = 5;
+        private const float circle_size = 24;
+
+        public Action<DragEvent> OnDragHandled;
 
         [UsedImplicitly]
         private readonly Bindable<double> startTime;
 
-        public Action<DragEvent> OnDragHandled;
+        private Bindable<int> indexInCurrentComboBindable;
+        private Bindable<int> comboIndexBindable;
 
+        private readonly Circle circle;
         private readonly DragBar dragBar;
-
         private readonly List<Container> shadowComponents = new List<Container>();
-
-        private DrawableHitObject drawableHitObject;
-
-        private Bindable<Color4> comboColour;
-
         private readonly Container mainComponents;
-
         private readonly OsuSpriteText comboIndexText;
 
-        private Bindable<int> comboIndex;
+        [Resolved]
-
+        private ISkinSource skin { get; set; }
-        private const float thickness = 5;
-
-        private const float shadow_radius = 5;
-
-        private const float circle_size = 24;
-
-        [Resolved(CanBeNull = true)]
-        private HitObjectComposer composer { get; set; }
 
         public TimelineHitObjectBlueprint(HitObject hitObject)
             : base(hitObject)
@@ -159,38 +149,38 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
         {
             base.LoadComplete();
 
-            if (composer != null)
-            {
-                // best effort to get the drawable representation for grabbing colour and what not.
-                drawableHitObject = composer.HitObjects.FirstOrDefault(d => d.HitObject == HitObject);
-            }
-
             if (HitObject is IHasComboInformation comboInfo)
             {
-                comboIndex = comboInfo.IndexInCurrentComboBindable.GetBoundCopy();
+                indexInCurrentComboBindable = comboInfo.IndexInCurrentComboBindable.GetBoundCopy();
-                comboIndex.BindValueChanged(combo =>
+                indexInCurrentComboBindable.BindValueChanged(_ => updateComboIndex(), true);
-                {
+
-                    comboIndexText.Text = (combo.NewValue + 1).ToString();
+                comboIndexBindable = comboInfo.ComboIndexBindable.GetBoundCopy();
-                }, true);
+                comboIndexBindable.BindValueChanged(_ => updateComboColour(), true);
+
+                skin.SourceChanged += updateComboColour;
             }
+        }
 
-            if (drawableHitObject != null)
+        private void updateComboIndex() => comboIndexText.Text = (indexInCurrentComboBindable.Value + 1).ToString();
-            {
-                comboColour = drawableHitObject.AccentColour.GetBoundCopy();
-                comboColour.BindValueChanged(colour =>
-                {
-                    if (HitObject is IHasDuration)
-                        mainComponents.Colour = ColourInfo.GradientHorizontal(drawableHitObject.AccentColour.Value, Color4.White);
-                    else
-                        mainComponents.Colour = drawableHitObject.AccentColour.Value;
 
-                    var col = mainComponents.Colour.TopLeft.Linear;
+        private void updateComboColour()
-                    float brightness = col.R + col.G + col.B;
+        {
+            if (!(HitObject is IHasComboInformation combo))
+                return;
 
-                    // decide the combo index colour based on brightness?
+            var comboColours = skin.GetConfig<GlobalSkinColours, IReadOnlyList<Color4>>(GlobalSkinColours.ComboColours)?.Value ?? Array.Empty<Color4>();
-                    comboIndexText.Colour = brightness > 0.5f ? Color4.Black : Color4.White;
+            var comboColour = combo.GetComboColour(comboColours);
-                }, true);
+
-            }
+            if (HitObject is IHasDuration)
+                mainComponents.Colour = ColourInfo.GradientHorizontal(comboColour, Color4.White);
+            else
+                mainComponents.Colour = comboColour;
+
+            var col = mainComponents.Colour.TopLeft.Linear;
+            float brightness = col.R + col.G + col.B;
+
+            // decide the combo index colour based on brightness?
+            comboIndexText.Colour = brightness > 0.5f ? Color4.Black : Color4.White;
         }
 
         protected override void Update()

osu.Game/Screens/Edit/Compose/ComposeScreen.cs

@@ -1,6 +1,7 @@
 // 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 System.Diagnostics;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -43,6 +44,21 @@ namespace osu.Game.Screens.Edit.Compose
             if (ruleset == null || composer == null)
                 return new ScreenWhiteBox.UnderConstructionMessage(ruleset == null ? "This beatmap" : $"{ruleset.Description}'s composer");
 
+            return wrapSkinnableContent(composer);
+        }
+
+        protected override Drawable CreateTimelineContent()
+        {
+            if (ruleset == null || composer == null)
+                return base.CreateTimelineContent();
+
+            return wrapSkinnableContent(new TimelineBlueprintContainer(composer));
+        }
+
+        private Drawable wrapSkinnableContent(Drawable content)
+        {
+            Debug.Assert(ruleset != null);
+
             var beatmapSkinProvider = new BeatmapSkinProvidingContainer(Beatmap.Value.Skin);
 
             // the beatmapSkinProvider is used as the fallback source here to allow the ruleset-specific skin implementation
@@ -51,9 +67,7 @@ namespace osu.Game.Screens.Edit.Compose
 
             // load the skinning hierarchy first.
             // this is intentionally done in two stages to ensure things are in a loaded state before exposing the ruleset to skin sources.
-            return beatmapSkinProvider.WithChild(rulesetSkinProvider.WithChild(composer));
+            return beatmapSkinProvider.WithChild(rulesetSkinProvider.WithChild(content));
         }
-
-        protected override Drawable CreateTimelineContent() => composer == null ? base.CreateTimelineContent() : new TimelineBlueprintContainer(composer);
     }
 }