Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support undo/redo for control points in the Editor #18668

Merged
merged 17 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion osu.Game.Rulesets.Osu/Mods/OsuModTarget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ private IEnumerable<double> getBeatsForTimingPoint(TimingControlPoint timingPoin
int i = 0;
double currentTime = timingPoint.Time;

while (!definitelyBigger(currentTime, mapEndTime) && controlPointInfo.TimingPointAt(currentTime) == timingPoint)
while (!definitelyBigger(currentTime, mapEndTime) && ReferenceEquals(controlPointInfo.TimingPointAt(currentTime), timingPoint))
{
beats.Add(Math.Floor(currentTime));
i++;
Expand Down
8 changes: 4 additions & 4 deletions osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ public void TestHitObjectPlacement()

// After placement these must be non-default as defaults are read-only.
AddAssert("Placed object has non-default control points", () =>
EditorBeatmap.HitObjects[0].SampleControlPoint != SampleControlPoint.DEFAULT &&
EditorBeatmap.HitObjects[0].DifficultyControlPoint != DifficultyControlPoint.DEFAULT);
!ReferenceEquals(EditorBeatmap.HitObjects[0].SampleControlPoint, SampleControlPoint.DEFAULT) &&
!ReferenceEquals(EditorBeatmap.HitObjects[0].DifficultyControlPoint, DifficultyControlPoint.DEFAULT));

ReloadEditorToSameBeatmap();

AddAssert("Beatmap still has correct timing point", () => EditorBeatmap.ControlPointInfo.TimingPoints.Single().Time == 500);

// After placement these must be non-default as defaults are read-only.
AddAssert("Placed object still has non-default control points", () =>
EditorBeatmap.HitObjects[0].SampleControlPoint != SampleControlPoint.DEFAULT &&
EditorBeatmap.HitObjects[0].DifficultyControlPoint != DifficultyControlPoint.DEFAULT);
!ReferenceEquals(EditorBeatmap.HitObjects[0].SampleControlPoint, SampleControlPoint.DEFAULT) &&
!ReferenceEquals(EditorBeatmap.HitObjects[0].DifficultyControlPoint, DifficultyControlPoint.DEFAULT));
}

[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ public TestBeatSyncedContainer()

private TimingControlPoint getNextTimingPoint(TimingControlPoint current)
{
if (timingPoints[^1] == current)
if (ReferenceEquals(timingPoints[^1], current))
return current;

int index = timingPoints.IndexOf(current); // -1 means that this is a "default beat"
Expand All @@ -281,7 +281,7 @@ private int calculateBeatCount(TimingControlPoint current)
{
if (timingPoints.Count == 0) return 0;

if (timingPoints[^1] == current)
if (ReferenceEquals(timingPoints[^1], current))
{
Debug.Assert(BeatSyncSource.Clock != null);

Expand Down
21 changes: 17 additions & 4 deletions osu.Game/Beatmaps/ControlPoints/ControlPoint.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// 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.

#nullable disable

using System;
using Newtonsoft.Json;
using osu.Game.Graphics;
Expand All @@ -11,7 +9,7 @@

namespace osu.Game.Beatmaps.ControlPoints
{
public abstract class ControlPoint : IComparable<ControlPoint>, IDeepCloneable<ControlPoint>
public abstract class ControlPoint : IComparable<ControlPoint>, IDeepCloneable<ControlPoint>, IEquatable<ControlPoint>
{
/// <summary>
/// The time at which the control point takes effect.
Expand All @@ -30,7 +28,7 @@ public abstract class ControlPoint : IComparable<ControlPoint>, IDeepCloneable<C
/// </summary>
/// <param name="existing">An existing control point to compare with.</param>
/// <returns>Whether this <see cref="ControlPoint"/> is redundant when placed alongside <paramref name="existing"/>.</returns>
public abstract bool IsRedundant(ControlPoint existing);
public abstract bool IsRedundant(ControlPoint? existing);

/// <summary>
/// Create an unbound copy of this control point.
Expand All @@ -48,5 +46,20 @@ public virtual void CopyFrom(ControlPoint other)
{
Time = other.Time;
}

public sealed override bool Equals(object? obj)
=> obj is ControlPoint otherControlPoint
&& Equals(otherControlPoint);

public virtual bool Equals(ControlPoint? other)
{
if (ReferenceEquals(other, null)) return false;
if (ReferenceEquals(other, this)) return true;

return Time == other.Time;
}

// ReSharper disable once NonReadonlyMemberInGetHashCode
public override int GetHashCode() => Time.GetHashCode();
}
}
26 changes: 21 additions & 5 deletions osu.Game/Beatmaps/ControlPoints/ControlPointGroup.cs
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
// 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.

#nullable disable

using System;
using System.Linq;
using osu.Framework.Bindables;

namespace osu.Game.Beatmaps.ControlPoints
{
public class ControlPointGroup : IComparable<ControlPointGroup>
public class ControlPointGroup : IComparable<ControlPointGroup>, IEquatable<ControlPointGroup>
{
public event Action<ControlPoint> ItemAdded;
public event Action<ControlPoint> ItemRemoved;
public event Action<ControlPoint>? ItemAdded;
public event Action<ControlPoint>? ItemRemoved;

/// <summary>
/// The time at which the control point takes effect.
Expand Down Expand Up @@ -48,5 +46,23 @@ public void Remove(ControlPoint point)
controlPoints.Remove(point);
ItemRemoved?.Invoke(point);
}

public sealed override bool Equals(object? obj)
=> obj is ControlPointGroup otherGroup
&& Equals(otherGroup);

public virtual bool Equals(ControlPointGroup? other)
=> other != null
&& Time == other.Time
&& ControlPoints.SequenceEqual(other.ControlPoints);

public override int GetHashCode()
{
HashCode hashCode = new HashCode();
hashCode.Add(Time);
foreach (var point in controlPoints)
hashCode.Add(point);
return hashCode.ToHashCode();
}
}
}
17 changes: 13 additions & 4 deletions osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs
Original file line number Diff line number Diff line change
@@ -1,8 +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.

#nullable disable

using System;
using osu.Framework.Bindables;
using osu.Game.Graphics;
using osuTK.Graphics;
Expand All @@ -12,7 +11,7 @@ namespace osu.Game.Beatmaps.ControlPoints
/// <remarks>
/// Note that going forward, this control point type should always be assigned directly to HitObjects.
/// </remarks>
public class DifficultyControlPoint : ControlPoint
public class DifficultyControlPoint : ControlPoint, IEquatable<DifficultyControlPoint>
{
public static readonly DifficultyControlPoint DEFAULT = new DifficultyControlPoint
{
Expand Down Expand Up @@ -41,7 +40,7 @@ public double SliderVelocity
set => SliderVelocityBindable.Value = value;
}

public override bool IsRedundant(ControlPoint existing)
public override bool IsRedundant(ControlPoint? existing)
=> existing is DifficultyControlPoint existingDifficulty
&& SliderVelocity == existingDifficulty.SliderVelocity;

Expand All @@ -51,5 +50,15 @@ public override void CopyFrom(ControlPoint other)

base.CopyFrom(other);
}

public override bool Equals(ControlPoint? other)
=> other is DifficultyControlPoint otherDifficultyControlPoint
&& Equals(otherDifficultyControlPoint);

public bool Equals(DifficultyControlPoint? other)
=> base.Equals(other)
&& SliderVelocity == other.SliderVelocity;

public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity);
}
}
19 changes: 15 additions & 4 deletions osu.Game/Beatmaps/ControlPoints/EffectControlPoint.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
// 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.

#nullable disable

using System;
using osu.Framework.Bindables;
using osu.Game.Graphics;
using osuTK.Graphics;

namespace osu.Game.Beatmaps.ControlPoints
{
public class EffectControlPoint : ControlPoint
public class EffectControlPoint : ControlPoint, IEquatable<EffectControlPoint>
{
public static readonly EffectControlPoint DEFAULT = new EffectControlPoint
{
Expand Down Expand Up @@ -68,7 +67,7 @@ public bool KiaiMode
set => KiaiModeBindable.Value = value;
}

public override bool IsRedundant(ControlPoint existing)
public override bool IsRedundant(ControlPoint? existing)
=> !OmitFirstBarLine
&& existing is EffectControlPoint existingEffect
&& KiaiMode == existingEffect.KiaiMode
Expand All @@ -83,5 +82,17 @@ public override void CopyFrom(ControlPoint other)

base.CopyFrom(other);
}

public override bool Equals(ControlPoint? other)
=> other is EffectControlPoint otherEffectControlPoint
&& Equals(otherEffectControlPoint);

public bool Equals(EffectControlPoint? other)
=> base.Equals(other)
&& OmitFirstBarLine == other.OmitFirstBarLine
&& ScrollSpeed == other.ScrollSpeed
&& KiaiMode == other.KiaiMode;

public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), OmitFirstBarLine, ScrollSpeed, KiaiMode);
}
}
18 changes: 14 additions & 4 deletions osu.Game/Beatmaps/ControlPoints/SampleControlPoint.cs
Original file line number Diff line number Diff line change
@@ -1,8 +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.

#nullable disable

using System;
using osu.Framework.Bindables;
using osu.Game.Audio;
using osu.Game.Graphics;
Expand All @@ -13,7 +12,7 @@ namespace osu.Game.Beatmaps.ControlPoints
/// <remarks>
/// Note that going forward, this control point type should always be assigned directly to HitObjects.
/// </remarks>
public class SampleControlPoint : ControlPoint
public class SampleControlPoint : ControlPoint, IEquatable<SampleControlPoint>
{
public const string DEFAULT_BANK = "normal";

Expand Down Expand Up @@ -73,7 +72,7 @@ public int SampleVolume
public virtual HitSampleInfo ApplyTo(HitSampleInfo hitSampleInfo)
=> hitSampleInfo.With(newBank: hitSampleInfo.Bank ?? SampleBank, newVolume: hitSampleInfo.Volume > 0 ? hitSampleInfo.Volume : SampleVolume);

public override bool IsRedundant(ControlPoint existing)
public override bool IsRedundant(ControlPoint? existing)
=> existing is SampleControlPoint existingSample
&& SampleBank == existingSample.SampleBank
&& SampleVolume == existingSample.SampleVolume;
Expand All @@ -85,5 +84,16 @@ public override void CopyFrom(ControlPoint other)

base.CopyFrom(other);
}

public override bool Equals(ControlPoint? other)
=> other is SampleControlPoint otherSampleControlPoint
&& Equals(otherSampleControlPoint);

public bool Equals(SampleControlPoint? other)
=> base.Equals(other)
&& SampleBank == other.SampleBank
&& SampleVolume == other.SampleVolume;

public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SampleBank, SampleVolume);
}
}
16 changes: 14 additions & 2 deletions osu.Game/Beatmaps/ControlPoints/TimingControlPoint.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
// 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;
using osu.Framework.Bindables;
using osu.Game.Beatmaps.Timing;
using osu.Game.Graphics;
using osuTK.Graphics;

namespace osu.Game.Beatmaps.ControlPoints
{
public class TimingControlPoint : ControlPoint
public class TimingControlPoint : ControlPoint, IEquatable<TimingControlPoint>
{
/// <summary>
/// The time signature at this control point.
Expand Down Expand Up @@ -68,7 +69,7 @@ public double BeatLength
public double BPM => 60000 / BeatLength;

// Timing points are never redundant as they can change the time signature.
public override bool IsRedundant(ControlPoint existing) => false;
public override bool IsRedundant(ControlPoint? existing) => false;

public override void CopyFrom(ControlPoint other)
{
Expand All @@ -77,5 +78,16 @@ public override void CopyFrom(ControlPoint other)

base.CopyFrom(other);
}

public override bool Equals(ControlPoint? other)
=> other is TimingControlPoint otherTimingControlPoint
&& Equals(otherTimingControlPoint);

public bool Equals(TimingControlPoint? other)
=> base.Equals(other)
&& TimeSignature.Equals(other.TimeSignature)
&& BeatLength.Equals(other.BeatLength);

public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), TimeSignature, BeatLength);
}
}
32 changes: 26 additions & 6 deletions osu.Game/Beatmaps/Formats/LegacyDecoder.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// 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.

#nullable disable

using System;
using System.Collections.Generic;
using osu.Framework.Extensions;
Expand Down Expand Up @@ -145,7 +143,7 @@ protected KeyValuePair<string, string> SplitKeyVal(string line, char separator =

protected string CleanFilename(string path) => path.Trim('"').ToStandardisedPath();

protected enum Section
public enum Section
{
General,
Editor,
Expand All @@ -162,7 +160,7 @@ protected enum Section
}

[Obsolete("Do not use unless you're a legacy ruleset and 100% sure.")]
public class LegacyDifficultyControlPoint : DifficultyControlPoint
public class LegacyDifficultyControlPoint : DifficultyControlPoint, IEquatable<LegacyDifficultyControlPoint>
{
/// <summary>
/// Legacy BPM multiplier that introduces floating-point errors for rulesets that depend on it.
Expand All @@ -188,9 +186,20 @@ public override void CopyFrom(ControlPoint other)

BpmMultiplier = ((LegacyDifficultyControlPoint)other).BpmMultiplier;
}

public override bool Equals(ControlPoint? other)
=> other is LegacyDifficultyControlPoint otherLegacyDifficultyControlPoint
&& Equals(otherLegacyDifficultyControlPoint);

public bool Equals(LegacyDifficultyControlPoint? other)
=> base.Equals(other)
&& BpmMultiplier == other.BpmMultiplier;

// ReSharper disable once NonReadonlyMemberInGetHashCode
public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BpmMultiplier);
}

internal class LegacySampleControlPoint : SampleControlPoint
internal class LegacySampleControlPoint : SampleControlPoint, IEquatable<LegacySampleControlPoint>
{
public int CustomSampleBank;

Expand All @@ -204,7 +213,7 @@ public override HitSampleInfo ApplyTo(HitSampleInfo hitSampleInfo)
return baseInfo;
}

public override bool IsRedundant(ControlPoint existing)
public override bool IsRedundant(ControlPoint? existing)
=> base.IsRedundant(existing)
&& existing is LegacySampleControlPoint existingSample
&& CustomSampleBank == existingSample.CustomSampleBank;
Expand All @@ -215,6 +224,17 @@ public override void CopyFrom(ControlPoint other)

CustomSampleBank = ((LegacySampleControlPoint)other).CustomSampleBank;
}

public override bool Equals(ControlPoint? other)
=> other is LegacySampleControlPoint otherLegacySampleControlPoint
&& Equals(otherLegacySampleControlPoint);

public bool Equals(LegacySampleControlPoint? other)
=> base.Equals(other)
&& CustomSampleBank == other.CustomSampleBank;

// ReSharper disable once NonReadonlyMemberInGetHashCode
public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), CustomSampleBank);
}
}
}
Loading