Merge pull request #2215 from peppy/fix-combo-colours

Fix beatmap combo colours not being applied

osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmapProcessor.cs

@@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Catch.Beatmaps
     {
         public override void PostProcess(Beatmap<CatchHitObject> beatmap)
         {
-            if (beatmap.ComboColors.Count == 0)
+            if (beatmap.ComboColours.Count == 0)
                 return;
 
             int index = 0;
@@ -31,11 +31,11 @@ namespace osu.Game.Rulesets.Catch.Beatmaps
                 if (obj.NewCombo)
                 {
                     if (lastObj != null) lastObj.LastInCombo = true;
-                    colourIndex = (colourIndex + 1) % beatmap.ComboColors.Count;
+                    colourIndex = (colourIndex + 1) % beatmap.ComboColours.Count;
                 }
 
                 obj.IndexInBeatmap = index++;
-                obj.ComboColour = beatmap.ComboColors[colourIndex];
+                obj.ComboColour = beatmap.ComboColours[colourIndex];
 
                 lastObj = obj;
             }

osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs

@@ -14,7 +14,7 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
         {
             applyStacking(beatmap);
 
-            if (beatmap.ComboColors.Count == 0)
+            if (beatmap.ComboColours.Count == 0)
                 return;
 
             int comboIndex = 0;
@@ -25,11 +25,11 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
                 if (obj.NewCombo)
                 {
                     comboIndex = 0;
-                    colourIndex = (colourIndex + 1) % beatmap.ComboColors.Count;
+                    colourIndex = (colourIndex + 1) % beatmap.ComboColours.Count;
                 }
 
                 obj.IndexInCurrentCombo = comboIndex++;
-                obj.ComboColour = beatmap.ComboColors[colourIndex];
+                obj.ComboColour = beatmap.ComboColours[colourIndex];
             }
         }
 

osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

@@ -167,7 +167,7 @@ namespace osu.Game.Tests.Beatmaps.Formats
             using (var resStream = Resource.OpenResource("Soleily - Renatus (Gamu) [Insane].osu"))
             using (var stream = new StreamReader(resStream))
             {
-                var comboColors = decoder.Decode(stream).ComboColors;
+                var comboColors = decoder.Decode(stream).ComboColours;
 
                 Color4[] expectedColors =
                 {

osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs

@@ -102,9 +102,9 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 new Color4(255, 187, 255, 255),
                 new Color4(255, 177, 140, 255),
             };
-            Assert.AreEqual(expected.Length, beatmap.ComboColors.Count);
+            Assert.AreEqual(expected.Length, beatmap.ComboColours.Count);
             for (int i = 0; i < expected.Length; i++)
-                Assert.AreEqual(expected[i], beatmap.ComboColors[i]);
+                Assert.AreEqual(expected[i], beatmap.ComboColours[i]);
         }
 
         [Test]

osu.Game/Beatmaps/Beatmap.cs

@@ -9,6 +9,7 @@ using System.Linq;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.IO.Serialization;
 using Newtonsoft.Json;
+using osu.Game.Beatmaps.Formats;
 using osu.Game.IO.Serialization.Converters;
 
 namespace osu.Game.Beatmaps
@@ -16,14 +17,14 @@ namespace osu.Game.Beatmaps
     /// <summary>
     /// A Beatmap containing converted HitObjects.
     /// </summary>
-    public class Beatmap<T> : IJsonSerializable
+    public class Beatmap<T> : IJsonSerializable, IHasComboColours
         where T : HitObject
     {
         public BeatmapInfo BeatmapInfo = new BeatmapInfo();
         public ControlPointInfo ControlPointInfo = new ControlPointInfo();
         public List<BreakPeriod> Breaks = new List<BreakPeriod>();
 
-        public List<Color4> ComboColors = new List<Color4>
+        public List<Color4> ComboColours { get; set; } = new List<Color4>
         {
             new Color4(17, 136, 170, 255),
             new Color4(102, 136, 0, 255),
@@ -55,7 +56,7 @@ namespace osu.Game.Beatmaps
             BeatmapInfo = original?.BeatmapInfo.DeepClone() ?? BeatmapInfo;
             ControlPointInfo = original?.ControlPointInfo ?? ControlPointInfo;
             Breaks = original?.Breaks ?? Breaks;
-            ComboColors = original?.ComboColors ?? ComboColors;
+            ComboColours = original?.ComboColours ?? ComboColours;
             HitObjects = original?.HitObjects ?? HitObjects;
 
             if (original == null && Metadata == null)

osu.Game/Beatmaps/BeatmapConverter.cs

@@ -50,9 +50,14 @@ namespace osu.Game.Beatmaps
         protected virtual Beatmap<T> ConvertBeatmap(Beatmap original)
         {
             var beatmap = CreateBeatmap();
+
+            // todo: this *must* share logic (or directly use) Beatmap<T>'s constructor.
+            // right now this isn't easily possible due to generic entanglement.
             beatmap.BeatmapInfo = original.BeatmapInfo;
             beatmap.ControlPointInfo = original.ControlPointInfo;
             beatmap.HitObjects = original.HitObjects.SelectMany(h => convert(h, original)).ToList();
+            beatmap.Breaks = original.Breaks;
+            beatmap.ComboColours = original.ComboColours;
 
             return beatmap;
         }

osu.Game/Beatmaps/Formats/IHasComboColours.cs

@@ -0,0 +1,13 @@
+// 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.Collections.Generic;
+using OpenTK.Graphics;
+
+namespace osu.Game.Beatmaps.Formats
+{
+    public interface IHasComboColours
+    {
+        List<Color4> ComboColours { get; set; }
+    }
+}

osu.Game/Beatmaps/Formats/IHasCustomColours.cs

@@ -0,0 +1,13 @@
+// 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.Collections.Generic;
+using OpenTK.Graphics;
+
+namespace osu.Game.Beatmaps.Formats
+{
+    public interface IHasCustomColours
+    {
+        Dictionary<string, Color4> CustomColours { get; set; }
+    }
+}

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -5,7 +5,6 @@ using System;
 using System.Globalization;
 using System.IO;
 using System.Linq;
-using OpenTK.Graphics;
 using osu.Game.Beatmaps.Timing;
 using osu.Game.Rulesets.Objects.Legacy;
 using osu.Game.Beatmaps.ControlPoints;
@@ -19,7 +18,6 @@ namespace osu.Game.Beatmaps.Formats
 
         private Beatmap beatmap;
 
-        private bool hasCustomColours;
         private ConvertHitObjectParser parser;
 
         private LegacySampleBank defaultSampleBank;
@@ -72,29 +70,28 @@ namespace osu.Game.Beatmaps.Formats
             {
                 case Section.General:
                     handleGeneral(line);
-                    break;
+                    return;
                 case Section.Editor:
                     handleEditor(line);
-                    break;
+                    return;
                 case Section.Metadata:
                     handleMetadata(line);
-                    break;
+                    return;
                 case Section.Difficulty:
                     handleDifficulty(line);
-                    break;
+                    return;
                 case Section.Events:
                     handleEvents(line);
-                    break;
+                    return;
                 case Section.TimingPoints:
                     handleTimingPoints(line);
-                    break;
+                    return;
-                case Section.Colours:
-                    handleColours(line);
-                    break;
                 case Section.HitObjects:
                     handleHitObjects(line);
-                    break;
+                    return;
             }
+
+            base.ParseLine(beatmap, section, line);
         }
 
         private void handleGeneral(string line)
@@ -364,38 +361,6 @@ namespace osu.Game.Beatmaps.Formats
             }
         }
 
-        private void handleColours(string line)
-        {
-            var pair = SplitKeyVal(line, ':');
-
-            string[] split = pair.Value.Split(',');
-
-            if (split.Length != 3)
-                throw new InvalidOperationException($@"Color specified in incorrect format (should be R,G,B): {pair.Value}");
-
-            byte r, g, b;
-            if (!byte.TryParse(split[0], out r) || !byte.TryParse(split[1], out g) || !byte.TryParse(split[2], out b))
-                throw new InvalidOperationException(@"Color must be specified with 8-bit integer components");
-
-            if (!hasCustomColours)
-            {
-                beatmap.ComboColors.Clear();
-                hasCustomColours = true;
-            }
-
-            // Note: the combo index specified in the beatmap is discarded
-            if (pair.Key.StartsWith(@"Combo"))
-            {
-                beatmap.ComboColors.Add(new Color4
-                {
-                    R = r / 255f,
-                    G = g / 255f,
-                    B = b / 255f,
-                    A = 1f,
-                });
-            }
-        }
-
         private void handleHitObjects(string line)
         {
             // If the ruleset wasn't specified, assume the osu!standard ruleset.

osu.Game/Beatmaps/Formats/LegacyDecoder.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using OpenTK.Graphics;
 
 namespace osu.Game.Beatmaps.Formats
 {
@@ -40,7 +41,53 @@ namespace osu.Game.Beatmaps.Formats
 
         protected virtual bool ShouldSkipLine(string line) => string.IsNullOrWhiteSpace(line) || line.StartsWith("//");
 
-        protected abstract void ParseLine(T output, Section section, string line);
+        protected virtual void ParseLine(T output, Section section, string line)
+        {
+            switch (section)
+            {
+                case Section.Colours:
+                    handleColours(output, line);
+                    return;
+            }
+        }
+
+        private bool hasComboColours;
+
+        private void handleColours(T output, string line)
+        {
+            var pair = SplitKeyVal(line, ':');
+
+            bool isCombo = pair.Key.StartsWith(@"Combo");
+
+            string[] split = pair.Value.Split(',');
+
+            if (split.Length != 3)
+                throw new InvalidOperationException($@"Color specified in incorrect format (should be R,G,B): {pair.Value}");
+
+            if (!byte.TryParse(split[0], out var r) || !byte.TryParse(split[1], out var g) || !byte.TryParse(split[2], out var b))
+                throw new InvalidOperationException(@"Color must be specified with 8-bit integer components");
+
+            Color4 colour = new Color4(r, g, b, 255);
+
+            if (isCombo)
+            {
+                if (!(output is IHasComboColours tHasComboColours)) return;
+
+                if (!hasComboColours)
+                {
+                    // remove default colours.
+                    tHasComboColours.ComboColours.Clear();
+                    hasComboColours = true;
+                }
+
+                tHasComboColours.ComboColours.Add(colour);
+            }
+            else
+            {
+                if (!(output is IHasCustomColours tHasCustomColours)) return;
+                tHasCustomColours.CustomColours[pair.Key] = colour;
+            }
+        }
 
         protected KeyValuePair<string, string> SplitKeyVal(string line, char separator)
         {
@@ -65,6 +112,7 @@ namespace osu.Game.Beatmaps.Formats
             Colours,
             HitObjects,
             Variables,
+            Fonts
         }
 
         internal enum LegacySampleBank

osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs

@@ -46,11 +46,13 @@ namespace osu.Game.Beatmaps.Formats
             {
                 case Section.Events:
                     handleEvents(line);
-                    break;
+                    return;
                 case Section.Variables:
                     handleVariables(line);
-                    break;
+                    return;
             }
+
+            base.ParseLine(storyboard, section, line);
         }
 
         private void handleEvents(string line)

osu.Game/osu.Game.csproj

@@ -267,6 +267,8 @@
     <Compile Include="Beatmaps\DifficultyCalculator.cs" />
     <Compile Include="Beatmaps\Drawables\BeatmapBackgroundSprite.cs" />
     <Compile Include="Beatmaps\Drawables\BeatmapSetCover.cs" />
+    <Compile Include="Beatmaps\Formats\IHasComboColours.cs" />
+    <Compile Include="Beatmaps\Formats\IHasCustomColours.cs" />
     <Compile Include="Beatmaps\Formats\JsonBeatmapDecoder.cs" />
     <Compile Include="Beatmaps\Formats\LegacyDecoder.cs" />
     <Compile Include="Beatmaps\Formats\LegacyStoryboardDecoder.cs" />