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.

Sign up for GitHub

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

Jump to bottom

Fix suggested value in audio offset adjust control being opposite in signs #26267

Merged
merged 3 commits into from
Dec 31, 2023
Merged

Fix suggested value in audio offset adjust control being opposite in signs #26267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc89390
Expose `SuggestedOffset` bindable for testing purposes
frenzibyte Dec 30, 2023
68dd103
Add failing test cases
frenzibyte Dec 30, 2023
e6fe631
Fix suggested value in audio offset adjust control being opposite in …
frenzibyte Dec 30, 2023
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
88 changes: 78 additions & 10 deletions osu.Game.Tests/Visual/Settings/TestSceneAudioOffsetAdjustControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Utils;
Expand All @@ -25,9 +25,15 @@ public partial class TestSceneAudioOffsetAdjustControl : OsuTestScene
private Container content = null!;
protected override Container Content => content;

private OsuConfigManager localConfig = null!;
private AudioOffsetAdjustControl adjustControl = null!;

[BackgroundDependencyLoader]
private void load()
{
localConfig = new OsuConfigManager(LocalStorage);
Dependencies.CacheAs(localConfig);

base.Content.AddRange(new Drawable[]
{
tracker,
Expand All @@ -41,23 +47,85 @@ private void load()
});
}

[Test]
public void TestBehaviour()
[SetUp]
public void SetUp() => Schedule(() =>
{
AddStep("create control", () => Child = new AudioOffsetAdjustControl
Child = adjustControl = new AudioOffsetAdjustControl
{
Current = new BindableDouble
{
MinValue = -500,
MaxValue = 500
}
});
Current = localConfig.GetBindable<double>(OsuSetting.AudioOffset),
};

localConfig.SetValue(OsuSetting.AudioOffset, 0.0);
tracker.ClearHistory();
});

[Test]
public void TestDisplay()
{
AddStep("set new score", () => statics.SetValue(Static.LastLocalUserScore, new ScoreInfo
{
HitEvents = TestSceneHitEventTimingDistributionGraph.CreateDistributedHitEvents(RNG.NextDouble(-100, 100)),
BeatmapInfo = Beatmap.Value.BeatmapInfo,
}));
AddStep("clear history", () => tracker.ClearHistory());
}

[Test]
public void TestBehaviour()
{
AddStep("set score with -20ms", () => setScore(-20));
AddAssert("suggested global offset is 20ms", () => adjustControl.SuggestedOffset.Value, () => Is.EqualTo(20));
AddStep("clear history", () => tracker.ClearHistory());

AddStep("set score with 40ms", () => setScore(40));
AddAssert("suggested global offset is -40ms", () => adjustControl.SuggestedOffset.Value, () => Is.EqualTo(-40));
AddStep("clear history", () => tracker.ClearHistory());
}

[Test]
public void TestNonZeroGlobalOffset()
{
AddStep("set global offset to -20ms", () => localConfig.SetValue(OsuSetting.AudioOffset, -20.0));
AddStep("set score with -20ms", () => setScore(-20));
AddAssert("suggested global offset is 0ms", () => adjustControl.SuggestedOffset.Value, () => Is.EqualTo(0));
AddStep("clear history", () => tracker.ClearHistory());

AddStep("set global offset to 20ms", () => localConfig.SetValue(OsuSetting.AudioOffset, 20.0));
AddStep("set score with 40ms", () => setScore(40));
AddAssert("suggested global offset is -20ms", () => adjustControl.SuggestedOffset.Value, () => Is.EqualTo(-20));
AddStep("clear history", () => tracker.ClearHistory());
}

[Test]
public void TestMultiplePlays()
{
AddStep("set score with -20ms", () => setScore(-20));
AddStep("set score with -10ms", () => setScore(-10));
AddAssert("suggested global offset is 15ms", () => adjustControl.SuggestedOffset.Value, () => Is.EqualTo(15));
AddStep("clear history", () => tracker.ClearHistory());

AddStep("set score with -20ms", () => setScore(-20));
AddStep("set global offset to 30ms", () => localConfig.SetValue(OsuSetting.AudioOffset, 30.0));
AddStep("set score with 10ms", () => setScore(10));
AddAssert("suggested global offset is 20ms", () => adjustControl.SuggestedOffset.Value, () => Is.EqualTo(20));
AddStep("clear history", () => tracker.ClearHistory());
}

private void setScore(double averageHitError)
{
statics.SetValue(Static.LastLocalUserScore, new ScoreInfo
{
HitEvents = TestSceneHitEventTimingDistributionGraph.CreateDistributedHitEvents(averageHitError),
BeatmapInfo = Beatmap.Value.BeatmapInfo,
});
}

protected override void Dispose(bool isDisposing)
{
if (localConfig.IsNotNull())
localConfig.Dispose();

base.Dispose(isDisposing);
}
}
}
18 changes: 10 additions & 8 deletions osu.Game/Overlays/Settings/Sections/Audio/AudioOffsetAdjustControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ namespace osu.Game.Overlays.Settings.Sections.Audio
{
public partial class AudioOffsetAdjustControl : SettingsItem<double>
{
public IBindable<double?> SuggestedOffset => ((AudioOffsetPreview)Control).SuggestedOffset;

[BackgroundDependencyLoader]
private void load()
{
Expand All @@ -44,7 +46,7 @@ public Bindable<double> Current

private readonly IBindableList<SessionAverageHitErrorTracker.DataPoint> averageHitErrorHistory = new BindableList<SessionAverageHitErrorTracker.DataPoint>();

private readonly Bindable<double?> suggestedOffset = new Bindable<double?>();
public readonly Bindable<double?> SuggestedOffset = new Bindable<double?>();

private Container<Box> notchContainer = null!;
private TextFlowContainer hintText = null!;
Expand Down Expand Up @@ -90,8 +92,8 @@ private void load(SessionAverageHitErrorTracker hitErrorTracker)
Text = "Apply suggested offset",
Action = () =>
{
if (suggestedOffset.Value.HasValue)
current.Value = suggestedOffset.Value.Value;
if (SuggestedOffset.Value.HasValue)
current.Value = SuggestedOffset.Value.Value;
hitErrorTracker.ClearHistory();
}
}
Expand All @@ -104,7 +106,7 @@ protected override void LoadComplete()
base.LoadComplete();

averageHitErrorHistory.BindCollectionChanged(updateDisplay, true);
suggestedOffset.BindValueChanged(_ => updateHintText(), true);
SuggestedOffset.BindValueChanged(_ => updateHintText(), true);
}

private void updateDisplay(object? _, NotifyCollectionChangedEventArgs e)
Expand Down Expand Up @@ -143,17 +145,17 @@ private void updateDisplay(object? _, NotifyCollectionChangedEventArgs e)
break;
}

suggestedOffset.Value = averageHitErrorHistory.Any() ? -averageHitErrorHistory.Average(dataPoint => dataPoint.SuggestedGlobalAudioOffset) : null;
SuggestedOffset.Value = averageHitErrorHistory.Any() ? averageHitErrorHistory.Average(dataPoint => dataPoint.SuggestedGlobalAudioOffset) : null;
}

private float getXPositionForOffset(double offset) => (float)(Math.Clamp(offset, current.MinValue, current.MaxValue) / (2 * current.MaxValue));

private void updateHintText()
{
hintText.Text = suggestedOffset.Value == null
hintText.Text = SuggestedOffset.Value == null
? @"Play a few beatmaps to receive a suggested offset!"
: $@"Based on the last {averageHitErrorHistory.Count} play(s), the suggested offset is {suggestedOffset.Value:N0} ms.";
applySuggestion.Enabled.Value = suggestedOffset.Value != null;
: $@"Based on the last {averageHitErrorHistory.Count} play(s), the suggested offset is {SuggestedOffset.Value:N0} ms.";
applySuggestion.Enabled.Value = SuggestedOffset.Value != null;
}
}
}
Expand Down
Loading