Merge pull request #22872 from Terochi/update-skinnable-sound-before-playing

Fix pause and fail screen samples not updating when skin is changed during gameplay
This commit is contained in:
Bartłomiej Dach 2023-04-05 23:39:10 +02:00 committed by GitHub
commit 92b8a0eaf4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 92 additions and 56 deletions

View File

@ -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 System.Linq;
@ -22,8 +20,10 @@ namespace osu.Game.Tests.Visual.Gameplay
{
public partial class TestSceneSkinnableSound : OsuTestScene
{
private TestSkinSourceContainer skinSource;
private PausableSkinnableSound skinnableSound;
private TestSkinSourceContainer skinSource = null!;
private PausableSkinnableSound skinnableSound = null!;
private const string sample_lookup = "Gameplay/normal-sliderslide";
[SetUpSteps]
public void SetUpSteps()
@ -36,7 +36,7 @@ namespace osu.Game.Tests.Visual.Gameplay
};
// has to be added after the hierarchy above else the `ISkinSource` dependency won't be cached.
skinSource.Add(skinnableSound = new PausableSkinnableSound(new SampleInfo("Gameplay/normal-sliderslide")));
skinSource.Add(skinnableSound = new PausableSkinnableSound(new SampleInfo(sample_lookup)));
});
}
@ -99,10 +99,28 @@ namespace osu.Game.Tests.Visual.Gameplay
AddAssert("sample not playing", () => !skinnableSound.IsPlaying);
}
[Test]
public void TestSampleUpdatedBeforePlaybackWhenNotPresent()
{
AddStep("make sample non-present", () => skinnableSound.Hide());
AddUntilStep("ensure not present", () => skinnableSound.IsPresent, () => Is.False);
AddUntilStep("ensure sample loaded", () => skinnableSound.ChildrenOfType<DrawableSample>().Single().Name, () => Is.EqualTo(sample_lookup));
AddStep("change source", () =>
{
skinSource.OverridingSample = new SampleVirtual("new skin");
skinSource.TriggerSourceChanged();
});
AddStep("start sample", () => skinnableSound.Play());
AddUntilStep("sample updated", () => skinnableSound.ChildrenOfType<DrawableSample>().Single().Name, () => Is.EqualTo("new skin"));
}
[Test]
public void TestSkinChangeDoesntPlayOnPause()
{
DrawableSample sample = null;
DrawableSample? sample = null;
AddStep("start sample", () =>
{
skinnableSound.Play();
@ -118,7 +136,7 @@ namespace osu.Game.Tests.Visual.Gameplay
AddAssert("retrieve and ensure current sample is different", () =>
{
DrawableSample oldSample = sample;
DrawableSample? oldSample = sample;
sample = skinnableSound.ChildrenOfType<DrawableSample>().Single();
return sample != oldSample;
});
@ -134,20 +152,29 @@ namespace osu.Game.Tests.Visual.Gameplay
private partial class TestSkinSourceContainer : Container, ISkinSource, ISamplePlaybackDisabler
{
[Resolved]
private ISkinSource source { get; set; }
private ISkinSource source { get; set; } = null!;
public event Action SourceChanged;
public event Action? SourceChanged;
public Bindable<bool> SamplePlaybackDisabled { get; } = new Bindable<bool>();
public ISample? OverridingSample;
IBindable<bool> ISamplePlaybackDisabler.SamplePlaybackDisabled => SamplePlaybackDisabled;
public Drawable GetDrawableComponent(ISkinComponentLookup lookup) => source?.GetDrawableComponent(lookup);
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => source?.GetTexture(componentName, wrapModeS, wrapModeT);
public ISample GetSample(ISampleInfo sampleInfo) => source?.GetSample(sampleInfo);
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => source?.GetConfig<TLookup, TValue>(lookup);
public ISkin FindProvider(Func<ISkin, bool> lookupFunction) => lookupFunction(this) ? this : source?.FindProvider(lookupFunction);
public IEnumerable<ISkin> AllSources => new[] { this }.Concat(source?.AllSources ?? Enumerable.Empty<ISkin>());
public Drawable? GetDrawableComponent(ISkinComponentLookup lookup) => source.GetDrawableComponent(lookup);
public Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => source.GetTexture(componentName, wrapModeS, wrapModeT);
public ISample? GetSample(ISampleInfo sampleInfo) => OverridingSample ?? source.GetSample(sampleInfo);
public IBindable<TValue>? GetConfig<TLookup, TValue>(TLookup lookup)
where TLookup : notnull
where TValue : notnull
{
return source.GetConfig<TLookup, TValue>(lookup);
}
public ISkin? FindProvider(Func<ISkin, bool> lookupFunction) => lookupFunction(this) ? this : source.FindProvider(lookupFunction);
public IEnumerable<ISkin> AllSources => new[] { this }.Concat(source.AllSources);
public void TriggerSourceChanged()
{

View File

@ -1,15 +1,13 @@
// 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 osu.Framework.Audio;
using osu.Framework.Bindables;
using osu.Game.Rulesets.UI;
using System;
using System.Collections.Generic;
using ManagedBass.Fx;
using osu.Framework.Allocation;
using osu.Framework.Audio.Sample;
using osu.Framework.Audio;
using osu.Framework.Audio.Track;
using osu.Framework.Bindables;
using osu.Framework.Extensions.Color4Extensions;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
@ -21,6 +19,7 @@ using osu.Game.Beatmaps;
using osu.Game.Configuration;
using osu.Game.Graphics;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.UI;
using osu.Game.Skinning;
using osuTK;
using osuTK.Graphics;
@ -50,8 +49,7 @@ namespace osu.Game.Screens.Play
private const float duration = 2500;
private ISample? failSample;
private SampleChannel? failSampleChannel;
private SkinnableSound failSample = null!;
[Resolved]
private OsuConfigManager config { get; set; } = null!;
@ -76,10 +74,10 @@ namespace osu.Game.Screens.Play
}
[BackgroundDependencyLoader]
private void load(AudioManager audio, ISkinSource skin, IBindable<WorkingBeatmap> beatmap)
private void load(AudioManager audio, IBindable<WorkingBeatmap> beatmap)
{
track = beatmap.Value.Track;
failSample = skin.GetSample(new SampleInfo(@"Gameplay/failsound"));
AddInternal(failSample = new SkinnableSound(new SampleInfo("Gameplay/failsound")));
AddRangeInternal(new Drawable[]
{
@ -126,7 +124,7 @@ namespace osu.Game.Screens.Play
failHighPassFilter.CutoffTo(300);
failLowPassFilter.CutoffTo(300, duration, Easing.OutCubic);
failSampleChannel = failSample?.Play();
failSample.Play();
track.AddAdjustment(AdjustableProperty.Frequency, trackFreq);
track.AddAdjustment(AdjustableProperty.Volume, volumeAdjustment);
@ -159,7 +157,7 @@ namespace osu.Game.Screens.Play
/// </summary>
public void Stop()
{
failSampleChannel?.Stop();
failSample.Stop();
removeFilters();
}

View File

@ -9,7 +9,6 @@ using JetBrains.Annotations;
using osu.Framework.Audio;
using osu.Framework.Audio.Sample;
using osu.Framework.Bindables;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Audio;
using osu.Framework.Graphics.Containers;
@ -70,20 +69,6 @@ namespace osu.Game.Skinning
updateSample();
}
protected override void LoadComplete()
{
base.LoadComplete();
CurrentSkin.SourceChanged += skinChangedImmediate;
}
private void skinChangedImmediate()
{
// Clean up the previous sample immediately on a source change.
// This avoids a potential call to Play() of an already disposed sample (samples are disposed along with the skin, but SkinChanged is scheduled).
clearPreviousSamples();
}
protected override void SkinChanged(ISkinSource skin)
{
base.SkinChanged(skin);
@ -109,6 +94,8 @@ namespace osu.Game.Skinning
private void updateSample()
{
clearPreviousSamples();
if (sampleInfo == null)
return;
@ -129,6 +116,8 @@ namespace osu.Game.Skinning
/// </summary>
public void Play()
{
FlushPendingSkinChanges();
if (Sample == null)
return;
@ -172,14 +161,6 @@ namespace osu.Game.Skinning
}
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
if (CurrentSkin.IsNotNull())
CurrentSkin.SourceChanged -= skinChangedImmediate;
}
#region Re-expose AudioContainer
public BindableNumber<double> Volume => sampleContainer.Volume;

View File

@ -5,6 +5,7 @@ using System;
using osu.Framework.Allocation;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Graphics.Pooling;
using osu.Framework.Threading;
namespace osu.Game.Skinning
{
@ -14,6 +15,8 @@ namespace osu.Game.Skinning
/// </summary>
public abstract partial class SkinReloadableDrawable : PoolableDrawable
{
private ScheduledDelegate? pendingSkinChange;
/// <summary>
/// Invoked when <see cref="CurrentSkin"/> has changed.
/// </summary>
@ -31,21 +34,30 @@ namespace osu.Game.Skinning
CurrentSkin.SourceChanged += onChange;
}
private void onChange() =>
// schedule required to avoid calls after disposed.
// note that this has the side-effect of components only performing a skin change when they are alive.
Scheduler.AddOnce(skinChanged);
protected override void LoadAsyncComplete()
{
base.LoadAsyncComplete();
skinChanged();
}
private void skinChanged()
/// <summary>
/// Force any pending <see cref="SkinChanged"/> calls to be performed immediately.
/// </summary>
/// <remarks>
/// When a skin change occurs, the handling provided by this class is scheduled.
/// In some cases, such a sample playback, this can result in the sample being played
/// just before it is updated to a potentially different sample.
///
/// Calling this method will ensure any pending update operations are run immediately.
/// It is recommended to call this before consuming the result of skin changes for anything non-drawable.
/// </remarks>
protected void FlushPendingSkinChanges()
{
SkinChanged(CurrentSkin);
OnSkinChanged?.Invoke();
if (pendingSkinChange == null)
return;
pendingSkinChange.RunTask();
pendingSkinChange = null;
}
/// <summary>
@ -56,6 +68,22 @@ namespace osu.Game.Skinning
{
}
private void onChange()
{
// schedule required to avoid calls after disposed.
// note that this has the side-effect of components only performing a skin change when they are alive.
pendingSkinChange?.Cancel();
pendingSkinChange = Scheduler.Add(skinChanged);
}
private void skinChanged()
{
SkinChanged(CurrentSkin);
OnSkinChanged?.Invoke();
pendingSkinChange = null;
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);

View File

@ -115,6 +115,8 @@ namespace osu.Game.Skinning
/// </summary>
public virtual void Play()
{
FlushPendingSkinChanges();
samplesContainer.ForEach(c =>
{
if (PlayWhenZeroVolume || c.AggregateVolume.Value > 0)