mirror of
https://github.com/ppy/osu.git
synced 2024-12-14 14:25:05 +08:00
Merge pull request #7784 from smoogipoo/fix-beatmap-disposal
Fix disposal-related errors by making WorkingBeatmap non-disposable
This commit is contained in:
commit
e022352812
@ -37,9 +37,9 @@ namespace osu.Game.Tests
|
||||
trackStore = audioManager.GetTrackStore(getZipReader());
|
||||
}
|
||||
|
||||
protected override void Dispose(bool isDisposing)
|
||||
~WaveformTestBeatmap()
|
||||
{
|
||||
base.Dispose(isDisposing);
|
||||
// Remove the track store from the audio manager
|
||||
trackStore?.Dispose();
|
||||
}
|
||||
|
||||
|
@ -36,8 +36,9 @@ namespace osu.Game.Beatmaps
|
||||
using (var stream = new LineBufferedReader(store.GetStream(getPathForFile(BeatmapInfo.Path))))
|
||||
return Decoder.GetDecoder<Beatmap>(stream).Decode(stream);
|
||||
}
|
||||
catch
|
||||
catch (Exception e)
|
||||
{
|
||||
Logger.Error(e, "Beatmap failed to load");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@ -59,8 +60,9 @@ namespace osu.Game.Beatmaps
|
||||
{
|
||||
return textureStore.Get(getPathForFile(Metadata.BackgroundFile));
|
||||
}
|
||||
catch
|
||||
catch (Exception e)
|
||||
{
|
||||
Logger.Error(e, "Background failed to load");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@ -74,8 +76,9 @@ namespace osu.Game.Beatmaps
|
||||
{
|
||||
return new VideoSprite(textureStore.GetStream(getPathForFile(Metadata.VideoFile)));
|
||||
}
|
||||
catch
|
||||
catch (Exception e)
|
||||
{
|
||||
Logger.Error(e, "Video failed to load");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@ -86,8 +89,9 @@ namespace osu.Game.Beatmaps
|
||||
{
|
||||
return (trackStore ??= AudioManager.GetTrackStore(store)).Get(getPathForFile(Metadata.AudioFile));
|
||||
}
|
||||
catch
|
||||
catch (Exception e)
|
||||
{
|
||||
Logger.Error(e, "Track failed to load");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@ -115,8 +119,9 @@ namespace osu.Game.Beatmaps
|
||||
var trackData = store.GetStream(getPathForFile(Metadata.AudioFile));
|
||||
return trackData == null ? null : new Waveform(trackData);
|
||||
}
|
||||
catch
|
||||
catch (Exception e)
|
||||
{
|
||||
Logger.Error(e, "Waveform failed to load");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
@ -17,10 +17,11 @@ using osu.Game.Rulesets.Objects.Types;
|
||||
using osu.Game.Rulesets.UI;
|
||||
using osu.Game.Skinning;
|
||||
using osu.Framework.Graphics.Video;
|
||||
using osu.Framework.Logging;
|
||||
|
||||
namespace osu.Game.Beatmaps
|
||||
{
|
||||
public abstract class WorkingBeatmap : IWorkingBeatmap, IDisposable
|
||||
public abstract class WorkingBeatmap : IWorkingBeatmap
|
||||
{
|
||||
public readonly BeatmapInfo BeatmapInfo;
|
||||
|
||||
@ -133,11 +134,29 @@ namespace osu.Game.Beatmaps
|
||||
return converted;
|
||||
}
|
||||
|
||||
public override string ToString() => BeatmapInfo.ToString();
|
||||
private CancellationTokenSource loadCancellation = new CancellationTokenSource();
|
||||
|
||||
public bool BeatmapLoaded => beatmapLoadTask?.IsCompleted ?? false;
|
||||
/// <summary>
|
||||
/// Beings loading the contents of this <see cref="WorkingBeatmap"/> asynchronously.
|
||||
/// </summary>
|
||||
public void BeginAsyncLoad()
|
||||
{
|
||||
loadBeatmapAsync();
|
||||
}
|
||||
|
||||
public Task<IBeatmap> LoadBeatmapAsync() => beatmapLoadTask ??= Task.Factory.StartNew(() =>
|
||||
/// <summary>
|
||||
/// Cancels the asynchronous loading of the contents of this <see cref="WorkingBeatmap"/>.
|
||||
/// </summary>
|
||||
public void CancelAsyncLoad()
|
||||
{
|
||||
loadCancellation?.Cancel();
|
||||
loadCancellation = new CancellationTokenSource();
|
||||
|
||||
if (beatmapLoadTask?.IsCompleted != true)
|
||||
beatmapLoadTask = null;
|
||||
}
|
||||
|
||||
private Task<IBeatmap> loadBeatmapAsync() => beatmapLoadTask ??= Task.Factory.StartNew(() =>
|
||||
{
|
||||
// Todo: Handle cancellation during beatmap parsing
|
||||
var b = GetBeatmap() ?? new Beatmap();
|
||||
@ -149,7 +168,11 @@ namespace osu.Game.Beatmaps
|
||||
b.BeatmapInfo = BeatmapInfo;
|
||||
|
||||
return b;
|
||||
}, beatmapCancellation.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
|
||||
}, loadCancellation.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
|
||||
|
||||
public override string ToString() => BeatmapInfo.ToString();
|
||||
|
||||
public bool BeatmapLoaded => beatmapLoadTask?.IsCompleted ?? false;
|
||||
|
||||
public IBeatmap Beatmap
|
||||
{
|
||||
@ -157,16 +180,25 @@ namespace osu.Game.Beatmaps
|
||||
{
|
||||
try
|
||||
{
|
||||
return LoadBeatmapAsync().Result;
|
||||
return loadBeatmapAsync().Result;
|
||||
}
|
||||
catch (TaskCanceledException)
|
||||
catch (AggregateException ae)
|
||||
{
|
||||
// This is the exception that is generally expected here, which occurs via natural cancellation of the asynchronous load
|
||||
if (ae.InnerExceptions.FirstOrDefault() is TaskCanceledException)
|
||||
return null;
|
||||
|
||||
Logger.Error(ae, "Beatmap failed to load");
|
||||
return null;
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
Logger.Error(e, "Beatmap failed to load");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private readonly CancellationTokenSource beatmapCancellation = new CancellationTokenSource();
|
||||
protected abstract IBeatmap GetBeatmap();
|
||||
private Task<IBeatmap> beatmapLoadTask;
|
||||
|
||||
@ -217,40 +249,11 @@ namespace osu.Game.Beatmaps
|
||||
/// </summary>
|
||||
public virtual void RecycleTrack() => track.Recycle();
|
||||
|
||||
#region Disposal
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
Dispose(true);
|
||||
GC.SuppressFinalize(this);
|
||||
}
|
||||
|
||||
private bool isDisposed;
|
||||
|
||||
protected virtual void Dispose(bool isDisposing)
|
||||
{
|
||||
if (isDisposed)
|
||||
return;
|
||||
|
||||
isDisposed = true;
|
||||
|
||||
// recycling logic is not here for the time being, as components which use
|
||||
// retrieved objects from WorkingBeatmap may not hold a reference to the WorkingBeatmap itself.
|
||||
// this should be fine as each retrieved component do have their own finalizers.
|
||||
|
||||
// cancelling the beatmap load is safe for now since the retrieval is a synchronous
|
||||
// operation. if we add an async retrieval method this may need to be reconsidered.
|
||||
beatmapCancellation?.Cancel();
|
||||
total_count.Value--;
|
||||
}
|
||||
|
||||
~WorkingBeatmap()
|
||||
{
|
||||
Dispose(false);
|
||||
total_count.Value--;
|
||||
}
|
||||
|
||||
#endregion
|
||||
|
||||
public class RecyclableLazy<T>
|
||||
{
|
||||
private Lazy<T> lazy;
|
||||
|
@ -401,15 +401,14 @@ namespace osu.Game
|
||||
if (nextBeatmap?.Track != null)
|
||||
nextBeatmap.Track.Completed += currentTrackCompleted;
|
||||
|
||||
using (var oldBeatmap = beatmap.OldValue)
|
||||
{
|
||||
if (oldBeatmap?.Track != null)
|
||||
oldBeatmap.Track.Completed -= currentTrackCompleted;
|
||||
}
|
||||
var oldBeatmap = beatmap.OldValue;
|
||||
if (oldBeatmap?.Track != null)
|
||||
oldBeatmap.Track.Completed -= currentTrackCompleted;
|
||||
|
||||
updateModDefaults();
|
||||
|
||||
nextBeatmap?.LoadBeatmapAsync();
|
||||
oldBeatmap?.CancelAsyncLoad();
|
||||
nextBeatmap?.BeginAsyncLoad();
|
||||
}
|
||||
|
||||
private void modsChanged(ValueChangedEvent<IReadOnlyList<Mod>> mods)
|
||||
|
@ -95,7 +95,7 @@ namespace osu.Game.Screens.Menu
|
||||
private void updateAmplitudes()
|
||||
{
|
||||
var track = beatmap.Value.TrackLoaded ? beatmap.Value.Track : null;
|
||||
var effect = beatmap.Value.BeatmapLoaded ? beatmap.Value.Beatmap.ControlPointInfo.EffectPointAt(track?.CurrentTime ?? Time.Current) : null;
|
||||
var effect = beatmap.Value.BeatmapLoaded ? beatmap.Value.Beatmap?.ControlPointInfo.EffectPointAt(track?.CurrentTime ?? Time.Current) : null;
|
||||
|
||||
float[] temporalAmplitudes = track?.CurrentAmplitudes.FrequencyAmplitudes;
|
||||
|
||||
|
@ -191,9 +191,9 @@ namespace osu.Game.Tests.Visual
|
||||
track = audio?.Tracks.GetVirtual(length);
|
||||
}
|
||||
|
||||
protected override void Dispose(bool isDisposing)
|
||||
~ClockBackedTestWorkingBeatmap()
|
||||
{
|
||||
base.Dispose(isDisposing);
|
||||
// Remove the track store from the audio manager
|
||||
store?.Dispose();
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user