diff --git a/osu.Game.Rulesets.Osu/Skinning/LegacyNewStyleSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/LegacyNewStyleSpinner.cs
index 72bc3ddc9a..739c87e037 100644
--- a/osu.Game.Rulesets.Osu/Skinning/LegacyNewStyleSpinner.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/LegacyNewStyleSpinner.cs
@@ -79,8 +79,8 @@ namespace osu.Game.Rulesets.Osu.Skinning
{
var spinner = (Spinner)drawableSpinner.HitObject;
- using (BeginAbsoluteSequence(spinner.StartTime - spinner.TimePreempt / 2, true))
- this.FadeInFromZero(spinner.TimePreempt / 2);
+ using (BeginAbsoluteSequence(spinner.StartTime - spinner.TimeFadeIn / 2, true))
+ this.FadeInFromZero(spinner.TimeFadeIn / 2);
fixedMiddle.FadeColour(Color4.White);
using (BeginAbsoluteSequence(spinner.StartTime, true))
diff --git a/osu.Game.Rulesets.Osu/Skinning/LegacyOldStyleSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/LegacyOldStyleSpinner.cs
index 0ae1d8f683..81a0df5ea5 100644
--- a/osu.Game.Rulesets.Osu/Skinning/LegacyOldStyleSpinner.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/LegacyOldStyleSpinner.cs
@@ -85,8 +85,8 @@ namespace osu.Game.Rulesets.Osu.Skinning
{
var spinner = drawableSpinner.HitObject;
- using (BeginAbsoluteSequence(spinner.StartTime - spinner.TimePreempt / 2, true))
- this.FadeInFromZero(spinner.TimePreempt / 2);
+ using (BeginAbsoluteSequence(spinner.StartTime - spinner.TimeFadeIn / 2, true))
+ this.FadeInFromZero(spinner.TimeFadeIn / 2);
}
protected override void Update()
diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs
index c4882046de..dca1e8a29e 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs
@@ -46,25 +46,37 @@ namespace osu.Game.Tests.Visual.Gameplay
///
/// If the test player should behave like the production one.
/// An action to run before player load but after bindable leases are returned.
- /// An action to run after container load.
- public void ResetPlayer(bool interactive, Action beforeLoadAction = null, Action afterLoadAction = null)
+ public void ResetPlayer(bool interactive, Action beforeLoadAction = null)
{
+ player = null;
+
audioManager.Volume.SetDefault();
InputManager.Clear();
+ container = new TestPlayerLoaderContainer(loader = new TestPlayerLoader(() => player = new TestPlayer(interactive, interactive)));
+
beforeLoadAction?.Invoke();
+
Beatmap.Value = CreateWorkingBeatmap(new OsuRuleset().RulesetInfo);
foreach (var mod in SelectedMods.Value.OfType())
mod.ApplyToTrack(MusicController.CurrentTrack);
- InputManager.Child = container = new TestPlayerLoaderContainer(
- loader = new TestPlayerLoader(() =>
- {
- afterLoadAction?.Invoke();
- return player = new TestPlayer(interactive, interactive);
- }));
+ InputManager.Child = container;
+ }
+
+ [Test]
+ public void TestEarlyExitBeforePlayerConstruction()
+ {
+ AddStep("load dummy beatmap", () => ResetPlayer(false, () => SelectedMods.Value = new[] { new OsuModNightcore() }));
+ AddUntilStep("wait for current", () => loader.IsCurrentScreen());
+ AddAssert("mod rate applied", () => MusicController.CurrentTrack.Rate != 1);
+ AddStep("exit loader", () => loader.Exit());
+ AddUntilStep("wait for not current", () => !loader.IsCurrentScreen());
+ AddAssert("player did not load", () => player == null);
+ AddUntilStep("player disposed", () => loader.DisposalTask == null);
+ AddAssert("mod rate still applied", () => MusicController.CurrentTrack.Rate != 1);
}
///
@@ -73,11 +85,12 @@ namespace osu.Game.Tests.Visual.Gameplay
/// speed adjustments were undone too late, causing cross-screen pollution.
///
[Test]
- public void TestEarlyExit()
+ public void TestEarlyExitAfterPlayerConstruction()
{
AddStep("load dummy beatmap", () => ResetPlayer(false, () => SelectedMods.Value = new[] { new OsuModNightcore() }));
AddUntilStep("wait for current", () => loader.IsCurrentScreen());
AddAssert("mod rate applied", () => MusicController.CurrentTrack.Rate != 1);
+ AddUntilStep("wait for non-null player", () => player != null);
AddStep("exit loader", () => loader.Exit());
AddUntilStep("wait for not current", () => !loader.IsCurrentScreen());
AddAssert("player did not load", () => !player.IsLoaded);
@@ -94,7 +107,7 @@ namespace osu.Game.Tests.Visual.Gameplay
AddUntilStep("wait for load ready", () =>
{
moveMouse();
- return player.LoadState == LoadState.Ready;
+ return player?.LoadState == LoadState.Ready;
});
AddRepeatStep("move mouse", moveMouse, 20);
@@ -195,19 +208,19 @@ namespace osu.Game.Tests.Visual.Gameplay
[Test]
public void TestMutedNotificationMasterVolume()
{
- addVolumeSteps("master volume", () => audioManager.Volume.Value = 0, null, () => audioManager.Volume.IsDefault);
+ addVolumeSteps("master volume", () => audioManager.Volume.Value = 0, () => audioManager.Volume.IsDefault);
}
[Test]
public void TestMutedNotificationTrackVolume()
{
- addVolumeSteps("music volume", () => audioManager.VolumeTrack.Value = 0, null, () => audioManager.VolumeTrack.IsDefault);
+ addVolumeSteps("music volume", () => audioManager.VolumeTrack.Value = 0, () => audioManager.VolumeTrack.IsDefault);
}
[Test]
public void TestMutedNotificationMuteButton()
{
- addVolumeSteps("mute button", null, () => container.VolumeOverlay.IsMuted.Value = true, () => !container.VolumeOverlay.IsMuted.Value);
+ addVolumeSteps("mute button", () => container.VolumeOverlay.IsMuted.Value = true, () => !container.VolumeOverlay.IsMuted.Value);
}
///
@@ -215,14 +228,13 @@ namespace osu.Game.Tests.Visual.Gameplay
///
/// What part of the volume system is checked
/// The action to be invoked to set the volume before loading
- /// The action to be invoked to set the volume after loading
/// The function to be invoked and checked
- private void addVolumeSteps(string volumeName, Action beforeLoad, Action afterLoad, Func assert)
+ private void addVolumeSteps(string volumeName, Action beforeLoad, Func assert)
{
AddStep("reset notification lock", () => sessionStatics.GetBindable(Static.MutedAudioNotificationShownOnce).Value = false);
- AddStep("load player", () => ResetPlayer(false, beforeLoad, afterLoad));
- AddUntilStep("wait for player", () => player.LoadState == LoadState.Ready);
+ AddStep("load player", () => ResetPlayer(false, beforeLoad));
+ AddUntilStep("wait for player", () => player?.LoadState == LoadState.Ready);
AddAssert("check for notification", () => container.NotificationOverlay.UnreadCount.Value == 1);
AddStep("click notification", () =>
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSongSelect.cs
index c62479faa0..3d225aa0a9 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSongSelect.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSongSelect.cs
@@ -16,7 +16,9 @@ using osu.Framework.Utils;
using osu.Game.Beatmaps;
using osu.Game.Online.Multiplayer;
using osu.Game.Rulesets;
+using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Mods;
using osu.Game.Screens.Multi.Components;
using osu.Game.Screens.Select;
@@ -145,6 +147,21 @@ namespace osu.Game.Tests.Visual.Multiplayer
AddAssert("new item has id 2", () => Room.Playlist.Last().ID == 2);
}
+ ///
+ /// Tests that the same instances are not shared between two playlist items.
+ ///
+ [Test]
+ public void TestNewItemHasNewModInstances()
+ {
+ AddStep("set dt mod", () => SelectedMods.Value = new[] { new OsuModDoubleTime() });
+ AddStep("create item", () => songSelect.BeatmapDetails.CreateNewItem());
+ AddStep("change mod rate", () => ((OsuModDoubleTime)SelectedMods.Value[0]).SpeedChange.Value = 2);
+ AddStep("create item", () => songSelect.BeatmapDetails.CreateNewItem());
+
+ AddAssert("item 1 has rate 1.5", () => Precision.AlmostEquals(1.5, ((OsuModDoubleTime)Room.Playlist.First().RequiredMods[0]).SpeedChange.Value));
+ AddAssert("item 2 has rate 2", () => Precision.AlmostEquals(2, ((OsuModDoubleTime)Room.Playlist.Last().RequiredMods[0]).SpeedChange.Value));
+ }
+
private class TestMatchSongSelect : MatchSongSelect
{
public new MatchBeatmapDetailArea BeatmapDetails => (MatchBeatmapDetailArea)base.BeatmapDetails;
diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs
index 7ff463361a..c5ce3751ef 100644
--- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs
+++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSettings.cs
@@ -8,6 +8,7 @@ using NUnit.Framework;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
+using osu.Framework.Utils;
using osu.Game.Beatmaps;
using osu.Game.Configuration;
using osu.Game.Graphics.UserInterface;
@@ -15,6 +16,7 @@ using osu.Game.Overlays.Mods;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Difficulty;
using osu.Game.Rulesets.Mods;
+using osu.Game.Rulesets.Osu.Mods;
using osu.Game.Rulesets.UI;
namespace osu.Game.Tests.Visual.UserInterface
@@ -75,6 +77,24 @@ namespace osu.Game.Tests.Visual.UserInterface
AddAssert("Customisation closed", () => modSelect.ModSettingsContainer.Alpha == 0);
}
+ [Test]
+ public void TestModSettingsUnboundWhenCopied()
+ {
+ OsuModDoubleTime original = null;
+ OsuModDoubleTime copy = null;
+
+ AddStep("create mods", () =>
+ {
+ original = new OsuModDoubleTime();
+ copy = (OsuModDoubleTime)original.CreateCopy();
+ });
+
+ AddStep("change property", () => original.SpeedChange.Value = 2);
+
+ AddAssert("original has new value", () => Precision.AlmostEquals(2.0, original.SpeedChange.Value));
+ AddAssert("copy has original value", () => Precision.AlmostEquals(1.5, copy.SpeedChange.Value));
+ }
+
private void createModSelect()
{
AddStep("create mod select", () =>
diff --git a/osu.Game/Rulesets/Mods/Mod.cs b/osu.Game/Rulesets/Mods/Mod.cs
index 0e5fe3fc9c..52ffa0ad2a 100644
--- a/osu.Game/Rulesets/Mods/Mod.cs
+++ b/osu.Game/Rulesets/Mods/Mod.cs
@@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
+using System.Diagnostics;
using System.Linq;
using System.Reflection;
using Newtonsoft.Json;
@@ -126,7 +127,25 @@ namespace osu.Game.Rulesets.Mods
///
/// Creates a copy of this initialised to a default state.
///
- public virtual Mod CreateCopy() => (Mod)MemberwiseClone();
+ public virtual Mod CreateCopy()
+ {
+ var copy = (Mod)Activator.CreateInstance(GetType());
+
+ // Copy bindable values across
+ foreach (var (_, prop) in this.GetSettingsSourceProperties())
+ {
+ var origBindable = prop.GetValue(this);
+ var copyBindable = prop.GetValue(copy);
+
+ // The bindables themselves are readonly, so the value must be transferred through the Bindable.Value property.
+ var valueProperty = origBindable.GetType().GetProperty(nameof(Bindable