1
0
mirror of https://github.com/ppy/osu.git synced 2025-01-19 12:22:57 +08:00

Merge pull request #11824 from peppy/fix-unsafe-skinnable-sample-play

Fix playing skinned samples being unsafe during skin changes
This commit is contained in:
Dean Herbert 2021-03-20 11:29:34 +09:00 committed by GitHub
commit b9b351311a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 127 additions and 61 deletions

View File

@ -140,7 +140,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy
return animation == null ? null : new LegacyManiaJudgementPiece(result, animation);
}
public override Sample GetSample(ISampleInfo sampleInfo)
public override ISample GetSample(ISampleInfo sampleInfo)
{
// layered hit sounds never play in mania
if (sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacySample && legacySample.IsLayered)

View File

@ -98,7 +98,7 @@ namespace osu.Game.Rulesets.Osu.Tests
return null;
}
public Sample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotImplementedException();

View File

@ -162,7 +162,7 @@ namespace osu.Game.Rulesets.Osu.Tests
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null;
public Sample GetSample(ISampleInfo sampleInfo) => null;
public ISample GetSample(ISampleInfo sampleInfo) => null;
public TValue GetValue<TConfiguration, TValue>(Func<TConfiguration, TValue> query) where TConfiguration : SkinConfiguration => default;
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => null;

View File

@ -152,7 +152,7 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy
throw new ArgumentOutOfRangeException(nameof(component), $"Invalid component type: {component}");
}
public override Sample GetSample(ISampleInfo sampleInfo) => Source.GetSample(new LegacyTaikoSampleInfo(sampleInfo));
public override ISample GetSample(ISampleInfo sampleInfo) => Source.GetSample(new LegacyTaikoSampleInfo(sampleInfo));
public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => Source.GetConfig<TLookup, TValue>(lookup);

View File

@ -121,7 +121,7 @@ namespace osu.Game.Tests.Gameplay
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => throw new NotImplementedException();
public Sample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
{

View File

@ -36,7 +36,7 @@ namespace osu.Game.Tests.Gameplay
public void TestRetrieveTopLevelSample()
{
ISkin skin = null;
Sample channel = null;
ISample channel = null;
AddStep("create skin", () => skin = new TestSkin("test-sample", this));
AddStep("retrieve sample", () => channel = skin.GetSample(new SampleInfo("test-sample")));
@ -48,7 +48,7 @@ namespace osu.Game.Tests.Gameplay
public void TestRetrieveSampleInSubFolder()
{
ISkin skin = null;
Sample channel = null;
ISample channel = null;
AddStep("create skin", () => skin = new TestSkin("folder/test-sample", this));
AddStep("retrieve sample", () => channel = skin.GetSample(new SampleInfo("folder/test-sample")));

View File

@ -59,7 +59,7 @@ namespace osu.Game.Tests.NonVisual.Skinning
}
public Drawable GetDrawableComponent(ISkinComponent component) => throw new NotSupportedException();
public Sample GetSample(ISampleInfo sampleInfo) => throw new NotSupportedException();
public ISample GetSample(ISampleInfo sampleInfo) => throw new NotSupportedException();
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotSupportedException();
}

View File

@ -219,7 +219,7 @@ namespace osu.Game.Tests.Skins
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => skin.GetTexture(componentName, wrapModeS, wrapModeT);
public Sample GetSample(ISampleInfo sampleInfo) => skin.GetSample(sampleInfo);
public ISample GetSample(ISampleInfo sampleInfo) => skin.GetSample(sampleInfo);
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => skin.GetConfig<TLookup, TValue>(lookup);
}

View File

@ -298,7 +298,7 @@ namespace osu.Game.Tests.Visual.Gameplay
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => throw new NotImplementedException();
public Sample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotImplementedException();
}
@ -309,7 +309,7 @@ namespace osu.Game.Tests.Visual.Gameplay
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => throw new NotImplementedException();
public Sample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotImplementedException();
}
@ -321,7 +321,7 @@ namespace osu.Game.Tests.Visual.Gameplay
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => throw new NotImplementedException();
public Sample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException();
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotImplementedException();

View File

@ -145,7 +145,7 @@ namespace osu.Game.Tests.Visual.Gameplay
public Drawable GetDrawableComponent(ISkinComponent component) => source?.GetDrawableComponent(component);
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => source?.GetTexture(componentName, wrapModeS, wrapModeT);
public Sample GetSample(ISampleInfo sampleInfo) => source?.GetSample(sampleInfo);
public ISample GetSample(ISampleInfo sampleInfo) => source?.GetSample(sampleInfo);
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => source?.GetConfig<TLookup, TValue>(lookup);
public void TriggerSourceChanged()

View File

@ -117,6 +117,10 @@ namespace osu.Game.Rulesets.UI
public void RemoveAllAdjustments(AdjustableProperty type) => throw new NotSupportedException();
public void BindAdjustments(IAggregateAudioAdjustment component) => throw new NotImplementedException();
public void UnbindAdjustments(IAggregateAudioAdjustment component) => throw new NotImplementedException();
public BindableNumber<double> Volume => throw new NotSupportedException();
public BindableNumber<double> Balance => throw new NotSupportedException();
@ -125,8 +129,6 @@ namespace osu.Game.Rulesets.UI
public BindableNumber<double> Tempo => throw new NotSupportedException();
public IBindable<double> GetAggregate(AdjustableProperty type) => throw new NotSupportedException();
public IBindable<double> AggregateVolume => throw new NotSupportedException();
public IBindable<double> AggregateBalance => throw new NotSupportedException();
@ -135,10 +137,6 @@ namespace osu.Game.Rulesets.UI
public IBindable<double> AggregateTempo => throw new NotSupportedException();
public void BindAdjustments(IAggregateAudioAdjustment component) => throw new NotImplementedException();
public void UnbindAdjustments(IAggregateAudioAdjustment component) => throw new NotImplementedException();
public int PlaybackConcurrency
{
get => throw new NotSupportedException();

View File

@ -24,7 +24,7 @@ namespace osu.Game.Skinning
public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null;
public override Sample GetSample(ISampleInfo sampleInfo) => null;
public override ISample GetSample(ISampleInfo sampleInfo) => null;
public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
{

View File

@ -48,7 +48,7 @@ namespace osu.Game.Skinning
/// <param name="sampleInfo">The requested sample.</param>
/// <returns>A matching sample channel, or null if unavailable.</returns>
[CanBeNull]
Sample GetSample(ISampleInfo sampleInfo);
ISample GetSample(ISampleInfo sampleInfo);
/// <summary>
/// Retrieve a configuration value.

View File

@ -39,7 +39,7 @@ namespace osu.Game.Skinning
return base.GetConfig<TLookup, TValue>(lookup);
}
public override Sample GetSample(ISampleInfo sampleInfo)
public override ISample GetSample(ISampleInfo sampleInfo)
{
if (sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacy && legacy.CustomSampleBank == 0)
{

View File

@ -7,6 +7,7 @@ using System.Diagnostics;
using System.IO;
using System.Linq;
using JetBrains.Annotations;
using osu.Framework.Audio;
using osu.Framework.Audio.Sample;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
@ -100,13 +101,6 @@ namespace osu.Game.Skinning
true) != null);
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
Textures?.Dispose();
Samples?.Dispose();
}
public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
{
switch (lookup)
@ -452,7 +446,7 @@ namespace osu.Game.Skinning
return null;
}
public override Sample GetSample(ISampleInfo sampleInfo)
public override ISample GetSample(ISampleInfo sampleInfo)
{
IEnumerable<string> lookupNames;
@ -468,7 +462,7 @@ namespace osu.Game.Skinning
var sample = Samples?.Get(lookup);
if (sample != null)
return sample;
return new LegacySkinSample(sample, this);
}
return null;
@ -504,5 +498,85 @@ namespace osu.Game.Skinning
string lastPiece = componentName.Split('/').Last();
yield return componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal) ? "taiko-" + lastPiece : lastPiece;
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
Textures?.Dispose();
Samples?.Dispose();
}
/// <summary>
/// A sample wrapper which keeps a reference to the contained skin to avoid finalizer garbage collection of the managing SampleStore.
/// </summary>
private class LegacySkinSample : ISample, IDisposable
{
private readonly Sample sample;
[UsedImplicitly]
private readonly LegacySkin skin;
public LegacySkinSample(Sample sample, LegacySkin skin)
{
this.sample = sample;
this.skin = skin;
}
public SampleChannel Play()
{
return sample.Play();
}
public SampleChannel GetChannel()
{
return sample.GetChannel();
}
public double Length => sample.Length;
public Bindable<int> PlaybackConcurrency => sample.PlaybackConcurrency;
public BindableNumber<double> Volume => sample.Volume;
public BindableNumber<double> Balance => sample.Balance;
public BindableNumber<double> Frequency => sample.Frequency;
public BindableNumber<double> Tempo => sample.Tempo;
public void BindAdjustments(IAggregateAudioAdjustment component)
{
sample.BindAdjustments(component);
}
public void UnbindAdjustments(IAggregateAudioAdjustment component)
{
sample.UnbindAdjustments(component);
}
public void AddAdjustment(AdjustableProperty type, IBindable<double> adjustBindable)
{
sample.AddAdjustment(type, adjustBindable);
}
public void RemoveAdjustment(AdjustableProperty type, IBindable<double> adjustBindable)
{
sample.RemoveAdjustment(type, adjustBindable);
}
public void RemoveAllAdjustments(AdjustableProperty type)
{
sample.RemoveAllAdjustments(type);
}
public IBindable<double> AggregateVolume => sample.AggregateVolume;
public IBindable<double> AggregateBalance => sample.AggregateBalance;
public IBindable<double> AggregateFrequency => sample.AggregateFrequency;
public IBindable<double> AggregateTempo => sample.AggregateTempo;
public void Dispose() => sample.Dispose();
}
}
}

View File

@ -34,7 +34,7 @@ namespace osu.Game.Skinning
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT)
=> Source.GetTexture(componentName, wrapModeS, wrapModeT);
public virtual Sample GetSample(ISampleInfo sampleInfo)
public virtual ISample GetSample(ISampleInfo sampleInfo)
{
if (!(sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacySample))
return Source.GetSample(sampleInfo);

View File

@ -86,21 +86,21 @@ namespace osu.Game.Skinning
sampleContainer.Clear();
Sample = null;
var ch = CurrentSkin.GetSample(sampleInfo);
var sample = CurrentSkin.GetSample(sampleInfo);
if (ch == null && AllowDefaultFallback)
if (sample == null && AllowDefaultFallback)
{
foreach (var lookup in sampleInfo.LookupNames)
{
if ((ch = sampleStore.Get(lookup)) != null)
if ((sample = sampleStore.Get(lookup)) != null)
break;
}
}
if (ch == null)
if (sample == null)
return;
sampleContainer.Add(Sample = new DrawableSample(ch));
sampleContainer.Add(Sample = new DrawableSample(sample));
// Start playback internally for the new sample if the previous one was playing beforehand.
if (wasPlaying && Looping)

View File

@ -19,7 +19,7 @@ namespace osu.Game.Skinning
public abstract Drawable GetDrawableComponent(ISkinComponent componentName);
public abstract Sample GetSample(ISampleInfo sampleInfo);
public abstract ISample GetSample(ISampleInfo sampleInfo);
public Texture GetTexture(string componentName) => GetTexture(componentName, default, default);

View File

@ -176,7 +176,7 @@ namespace osu.Game.Skinning
public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => CurrentSkin.Value.GetTexture(componentName, wrapModeS, wrapModeT);
public Sample GetSample(ISampleInfo sampleInfo) => CurrentSkin.Value.GetSample(sampleInfo);
public ISample GetSample(ISampleInfo sampleInfo) => CurrentSkin.Value.GetSample(sampleInfo);
public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => CurrentSkin.Value.GetConfig<TLookup, TValue>(lookup);

View File

@ -59,9 +59,9 @@ namespace osu.Game.Skinning
return fallbackSource?.GetTexture(componentName, wrapModeS, wrapModeT);
}
public Sample GetSample(ISampleInfo sampleInfo)
public ISample GetSample(ISampleInfo sampleInfo)
{
Sample sourceChannel;
ISample sourceChannel;
if (AllowSampleLookup(sampleInfo) && (sourceChannel = skin?.GetSample(sampleInfo)) != null)
return sourceChannel;

View File

@ -131,6 +131,15 @@ namespace osu.Game.Skinning
});
}
protected override void LoadAsyncComplete()
{
// ensure samples are constructed before SkinChanged() is called via base.LoadAsyncComplete().
if (!samplesContainer.Any())
updateSamples();
base.LoadAsyncComplete();
}
/// <summary>
/// Stops the samples.
/// </summary>
@ -139,12 +148,6 @@ namespace osu.Game.Skinning
samplesContainer.ForEach(c => c.Stop());
}
protected override void SkinChanged(ISkinSource skin, bool allowFallback)
{
base.SkinChanged(skin, allowFallback);
updateSamples();
}
private void updateSamples()
{
bool wasPlaying = IsPlaying;
@ -176,24 +179,15 @@ namespace osu.Game.Skinning
public BindableNumber<double> Tempo => samplesContainer.Tempo;
public void BindAdjustments(IAggregateAudioAdjustment component)
{
samplesContainer.BindAdjustments(component);
}
public void BindAdjustments(IAggregateAudioAdjustment component) => samplesContainer.BindAdjustments(component);
public void UnbindAdjustments(IAggregateAudioAdjustment component)
{
samplesContainer.UnbindAdjustments(component);
}
public void UnbindAdjustments(IAggregateAudioAdjustment component) => samplesContainer.UnbindAdjustments(component);
public void AddAdjustment(AdjustableProperty type, IBindable<double> adjustBindable)
=> samplesContainer.AddAdjustment(type, adjustBindable);
public void AddAdjustment(AdjustableProperty type, IBindable<double> adjustBindable) => samplesContainer.AddAdjustment(type, adjustBindable);
public void RemoveAdjustment(AdjustableProperty type, IBindable<double> adjustBindable)
=> samplesContainer.RemoveAdjustment(type, adjustBindable);
public void RemoveAdjustment(AdjustableProperty type, IBindable<double> adjustBindable) => samplesContainer.RemoveAdjustment(type, adjustBindable);
public void RemoveAllAdjustments(AdjustableProperty type)
=> samplesContainer.RemoveAllAdjustments(type);
public void RemoveAllAdjustments(AdjustableProperty type) => samplesContainer.RemoveAllAdjustments(type);
/// <summary>
/// Whether any samples are currently playing.