From 3c028ce05cd619c58488793e9e26548ae07aeff3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 19 Jul 2021 12:38:22 +0900 Subject: [PATCH] Add `IDeepCloneable` interface and update existing `CreateCopy` methods to use it --- CodeAnalysis/BannedSymbols.txt | 1 + osu.Game.Tests/NonVisual/ControlPointInfoTest.cs | 4 ++-- .../Visual/UserInterface/TestSceneModSettings.cs | 4 ++-- osu.Game/Beatmaps/ControlPoints/ControlPoint.cs | 5 +++-- .../Beatmaps/ControlPoints/ControlPointInfo.cs | 7 ++++--- osu.Game/Online/Rooms/Room.cs | 5 +++-- osu.Game/Overlays/Mods/ModSelectOverlay.cs | 2 +- .../Rulesets/Difficulty/DifficultyCalculator.cs | 2 +- osu.Game/Rulesets/Mods/Mod.cs | 4 ++-- osu.Game/Rulesets/Mods/MultiMod.cs | 2 +- osu.Game/Screens/Edit/Editor.cs | 2 +- .../OnlinePlay/Lounge/Components/DrawableRoom.cs | 2 +- .../Screens/OnlinePlay/OnlinePlaySongSelect.cs | 8 ++++---- .../OnlinePlay/Playlists/PlaylistsSongSelect.cs | 4 ++-- osu.Game/Screens/Play/Player.cs | 2 +- osu.Game/Utils/IDeepCloneable.cs | 16 ++++++++++++++++ 16 files changed, 45 insertions(+), 25 deletions(-) create mode 100644 osu.Game/Utils/IDeepCloneable.cs diff --git a/CodeAnalysis/BannedSymbols.txt b/CodeAnalysis/BannedSymbols.txt index 46c50dbfa2..ea3e25142c 100644 --- a/CodeAnalysis/BannedSymbols.txt +++ b/CodeAnalysis/BannedSymbols.txt @@ -3,6 +3,7 @@ M:System.Object.Equals(System.Object)~System.Boolean;Don't use object.Equals. Us M:System.ValueType.Equals(System.Object)~System.Boolean;Don't use object.Equals(Fallbacks to ValueType). Use IEquatable or EqualityComparer.Default instead. M:System.Nullable`1.Equals(System.Object)~System.Boolean;Use == instead. T:System.IComparable;Don't use non-generic IComparable. Use generic version instead. +T:SixLabors.ImageSharp.IDeepCloneable`1;Use osu.Game.Utils.IDeepCloneable instead. M:osu.Framework.Graphics.Sprites.SpriteText.#ctor;Use OsuSpriteText. M:osu.Framework.Bindables.IBindableList`1.GetBoundCopy();Fails on iOS. Use manual ctor + BindTo instead. (see https://github.com/mono/mono/issues/19900) T:Microsoft.EntityFrameworkCore.Internal.EnumerableExtensions;Don't use internal extension methods. diff --git a/osu.Game.Tests/NonVisual/ControlPointInfoTest.cs b/osu.Game.Tests/NonVisual/ControlPointInfoTest.cs index b27c257795..240ae4a90c 100644 --- a/osu.Game.Tests/NonVisual/ControlPointInfoTest.cs +++ b/osu.Game.Tests/NonVisual/ControlPointInfoTest.cs @@ -248,13 +248,13 @@ namespace osu.Game.Tests.NonVisual } [Test] - public void TestCreateCopyIsDeepClone() + public void TestDeepClone() { var cpi = new ControlPointInfo(); cpi.Add(1000, new TimingControlPoint { BeatLength = 500 }); - var cpiCopy = cpi.CreateCopy(); + var cpiCopy = cpi.DeepClone(); cpiCopy.Add(2000, new TimingControlPoint { BeatLength = 500 }); diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs index bda1973354..65db2e9644 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs @@ -88,7 +88,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("create mods", () => { original = new OsuModDoubleTime(); - copy = (OsuModDoubleTime)original.CreateCopy(); + copy = (OsuModDoubleTime)original.DeepClone(); }); AddStep("change property", () => original.SpeedChange.Value = 2); @@ -106,7 +106,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("create mods", () => { original = new MultiMod(new OsuModDoubleTime()); - copy = (MultiMod)original.CreateCopy(); + copy = (MultiMod)original.DeepClone(); }); AddStep("change property", () => ((OsuModDoubleTime)original.Mods[0]).SpeedChange.Value = 2); diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPoint.cs b/osu.Game/Beatmaps/ControlPoints/ControlPoint.cs index e8dc623ddb..643c5d9adb 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPoint.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPoint.cs @@ -3,11 +3,12 @@ using System; using osu.Game.Graphics; +using osu.Game.Utils; using osuTK.Graphics; namespace osu.Game.Beatmaps.ControlPoints { - public abstract class ControlPoint : IComparable + public abstract class ControlPoint : IComparable, IDeepCloneable { /// /// The time at which the control point takes effect. @@ -32,7 +33,7 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// Create an unbound copy of this control point. /// - public ControlPoint CreateCopy() + public ControlPoint DeepClone() { var copy = (ControlPoint)Activator.CreateInstance(GetType()); diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 25d0843a71..2d0fc17a7b 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -10,11 +10,12 @@ using osu.Framework.Bindables; using osu.Framework.Lists; using osu.Framework.Utils; using osu.Game.Screens.Edit; +using osu.Game.Utils; namespace osu.Game.Beatmaps.ControlPoints { [Serializable] - public class ControlPointInfo + public class ControlPointInfo : IDeepCloneable { /// /// All control points grouped by time. @@ -350,12 +351,12 @@ namespace osu.Game.Beatmaps.ControlPoints } } - public ControlPointInfo CreateCopy() + public ControlPointInfo DeepClone() { var controlPointInfo = new ControlPointInfo(); foreach (var point in AllControlPoints) - controlPointInfo.Add(point.Time, point.CreateCopy()); + controlPointInfo.Add(point.Time, point.DeepClone()); return controlPointInfo; } diff --git a/osu.Game/Online/Rooms/Room.cs b/osu.Game/Online/Rooms/Room.cs index b28680ffef..863fefd55c 100644 --- a/osu.Game/Online/Rooms/Room.cs +++ b/osu.Game/Online/Rooms/Room.cs @@ -10,10 +10,11 @@ using osu.Game.IO.Serialization.Converters; using osu.Game.Online.Rooms.GameTypes; using osu.Game.Online.Rooms.RoomStatuses; using osu.Game.Users; +using osu.Game.Utils; namespace osu.Game.Online.Rooms { - public class Room + public class Room : IDeepCloneable { [Cached] [JsonProperty("id")] @@ -120,7 +121,7 @@ namespace osu.Game.Online.Rooms /// Create a copy of this room without online information. /// Should be used to create a local copy of a room for submitting in the future. /// - public Room CreateCopy() + public Room DeepClone() { var copy = new Room(); diff --git a/osu.Game/Overlays/Mods/ModSelectOverlay.cs b/osu.Game/Overlays/Mods/ModSelectOverlay.cs index 98a79a62c8..793bb79318 100644 --- a/osu.Game/Overlays/Mods/ModSelectOverlay.cs +++ b/osu.Game/Overlays/Mods/ModSelectOverlay.cs @@ -429,7 +429,7 @@ namespace osu.Game.Overlays.Mods if (!Stacked) modEnumeration = ModUtils.FlattenMods(modEnumeration); - section.Mods = modEnumeration.Select(getValidModOrNull).Where(m => m != null).Select(m => m.CreateCopy()); + section.Mods = modEnumeration.Select(getValidModOrNull).Where(m => m != null).Select(m => m.DeepClone()); } updateSelectedButtons(); diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index 3cc69bd85b..224c9178ae 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Difficulty /// A structure describing the difficulty of the beatmap. public DifficultyAttributes Calculate(params Mod[] mods) { - mods = mods.Select(m => m.CreateCopy()).ToArray(); + mods = mods.Select(m => m.DeepClone()).ToArray(); IBeatmap playableBeatmap = beatmap.GetPlayableBeatmap(ruleset.RulesetInfo, mods); diff --git a/osu.Game/Rulesets/Mods/Mod.cs b/osu.Game/Rulesets/Mods/Mod.cs index 6f00bb6c75..f2fd02c652 100644 --- a/osu.Game/Rulesets/Mods/Mod.cs +++ b/osu.Game/Rulesets/Mods/Mod.cs @@ -21,7 +21,7 @@ namespace osu.Game.Rulesets.Mods /// The base class for gameplay modifiers. /// [ExcludeFromDynamicCompile] - public abstract class Mod : IMod, IEquatable, IJsonSerializable + public abstract class Mod : IMod, IEquatable, IJsonSerializable, IDeepCloneable { /// /// The name of this mod. @@ -132,7 +132,7 @@ namespace osu.Game.Rulesets.Mods /// /// Creates a copy of this initialised to a default state. /// - public virtual Mod CreateCopy() + public virtual Mod DeepClone() { var result = (Mod)Activator.CreateInstance(GetType()); result.CopyFrom(this); diff --git a/osu.Game/Rulesets/Mods/MultiMod.cs b/osu.Game/Rulesets/Mods/MultiMod.cs index 2107009dbb..1c41c6b8b3 100644 --- a/osu.Game/Rulesets/Mods/MultiMod.cs +++ b/osu.Game/Rulesets/Mods/MultiMod.cs @@ -20,7 +20,7 @@ namespace osu.Game.Rulesets.Mods Mods = mods; } - public override Mod CreateCopy() => new MultiMod(Mods.Select(m => m.CreateCopy()).ToArray()); + public override Mod DeepClone() => new MultiMod(Mods.Select(m => m.DeepClone()).ToArray()); public override Type[] IncompatibleMods => Mods.SelectMany(m => m.IncompatibleMods).ToArray(); } diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 986a4efb28..71dd47b058 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -128,7 +128,7 @@ namespace osu.Game.Screens.Edit // clone these locally for now to avoid incurring overhead on GetPlayableBeatmap usages. // eventually we will want to improve how/where this is done as there are issues with *not* cloning it in all cases. - playableBeatmap.ControlPointInfo = playableBeatmap.ControlPointInfo.CreateCopy(); + playableBeatmap.ControlPointInfo = playableBeatmap.ControlPointInfo.DeepClone(); } catch (Exception e) { diff --git a/osu.Game/Screens/OnlinePlay/Lounge/Components/DrawableRoom.cs b/osu.Game/Screens/OnlinePlay/Lounge/Components/DrawableRoom.cs index 35782c6104..b4b1a09c4f 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/Components/DrawableRoom.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/Components/DrawableRoom.cs @@ -242,7 +242,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge.Components { new OsuMenuItem("Create copy", MenuItemType.Standard, () => { - parentScreen?.OpenNewRoom(Room.CreateCopy()); + parentScreen?.OpenNewRoom(Room.DeepClone()); }) }; } diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlaySongSelect.cs b/osu.Game/Screens/OnlinePlay/OnlinePlaySongSelect.cs index 2c46f76737..be28de5c43 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlaySongSelect.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlaySongSelect.cs @@ -72,8 +72,8 @@ namespace osu.Game.Screens.OnlinePlay // At this point, Mods contains both the required and allowed mods. For selection purposes, it should only contain the required mods. // Similarly, freeMods is currently empty but should only contain the allowed mods. - Mods.Value = selectedItem?.Value?.RequiredMods.Select(m => m.CreateCopy()).ToArray() ?? Array.Empty(); - FreeMods.Value = selectedItem?.Value?.AllowedMods.Select(m => m.CreateCopy()).ToArray() ?? Array.Empty(); + Mods.Value = selectedItem?.Value?.RequiredMods.Select(m => m.DeepClone()).ToArray() ?? Array.Empty(); + FreeMods.Value = selectedItem?.Value?.AllowedMods.Select(m => m.DeepClone()).ToArray() ?? Array.Empty(); Mods.BindValueChanged(onModsChanged); Ruleset.BindValueChanged(onRulesetChanged); @@ -108,8 +108,8 @@ namespace osu.Game.Screens.OnlinePlay } }; - item.RequiredMods.AddRange(Mods.Value.Select(m => m.CreateCopy())); - item.AllowedMods.AddRange(FreeMods.Value.Select(m => m.CreateCopy())); + item.RequiredMods.AddRange(Mods.Value.Select(m => m.DeepClone())); + item.AllowedMods.AddRange(FreeMods.Value.Select(m => m.DeepClone())); SelectItem(item); return true; diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsSongSelect.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsSongSelect.cs index 21335fc90c..076fa77336 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsSongSelect.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsSongSelect.cs @@ -55,10 +55,10 @@ namespace osu.Game.Screens.OnlinePlay.Playlists item.Ruleset.Value = Ruleset.Value; item.RequiredMods.Clear(); - item.RequiredMods.AddRange(Mods.Value.Select(m => m.CreateCopy())); + item.RequiredMods.AddRange(Mods.Value.Select(m => m.DeepClone())); item.AllowedMods.Clear(); - item.AllowedMods.AddRange(FreeMods.Value.Select(m => m.CreateCopy())); + item.AllowedMods.AddRange(FreeMods.Value.Select(m => m.DeepClone())); } } } diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 97854ee12f..51f1dbd121 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -184,7 +184,7 @@ namespace osu.Game.Screens.Play [BackgroundDependencyLoader(true)] private void load(AudioManager audio, OsuConfigManager config, OsuGameBase game) { - Mods.Value = base.Mods.Value.Select(m => m.CreateCopy()).ToArray(); + Mods.Value = base.Mods.Value.Select(m => m.DeepClone()).ToArray(); if (Beatmap.Value is DummyWorkingBeatmap) return; diff --git a/osu.Game/Utils/IDeepCloneable.cs b/osu.Game/Utils/IDeepCloneable.cs new file mode 100644 index 0000000000..6877f346c4 --- /dev/null +++ b/osu.Game/Utils/IDeepCloneable.cs @@ -0,0 +1,16 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Utils +{ + /// A generic interface for a deeply cloneable type. + /// The type of object to clone. + public interface IDeepCloneable where T : class + { + /// + /// Creates a new that is a deep copy of the current instance. + /// + /// The . + T DeepClone(); + } +}