mirror of
https://github.com/ppy/osu
synced 2024-12-25 16:22:23 +00:00
Improve safety of ongoing operation tracker
Finishing an operation started via `OngoingOperationTracker.BeginOperation()` was risky in cases where the operation ended at a callback on another thread (which, in the case of multiplayer, is *most* cases). In particular, if any consumer registered a callback that mutates transforms when the operation ends, it would result in crashes after the framework-side safety checks. Rework `OngoingOperationTracker` into an always-present component residing in the drawable hierarchy, and ensure that the `operationInProgress` bindable is always updated on the update thread. This way consumers don't have to add local schedules in multiple places.
This commit is contained in:
parent
c8d83a9fb3
commit
8c3955d341
@ -3,18 +3,13 @@
|
||||
|
||||
using System;
|
||||
using NUnit.Framework;
|
||||
using osu.Framework.Allocation;
|
||||
using osu.Framework.Graphics;
|
||||
using osu.Game.Screens.OnlinePlay;
|
||||
using osu.Game.Screens.OnlinePlay.Multiplayer;
|
||||
|
||||
namespace osu.Game.Tests.Visual.Multiplayer
|
||||
{
|
||||
public class TestSceneCreateMultiplayerMatchButton : MultiplayerTestScene
|
||||
{
|
||||
[Cached]
|
||||
private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker();
|
||||
|
||||
private CreateMultiplayerMatchButton button;
|
||||
|
||||
public override void SetUpSteps()
|
||||
@ -36,7 +31,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
|
||||
|
||||
assertButtonEnableState(true);
|
||||
|
||||
AddStep("begin joining room", () => joiningRoomOperation = ongoingOperationTracker.BeginOperation());
|
||||
AddStep("begin joining room", () => joiningRoomOperation = OngoingOperationTracker.BeginOperation());
|
||||
assertButtonEnableState(false);
|
||||
|
||||
AddStep("end joining room", () => joiningRoomOperation.Dispose());
|
||||
|
@ -4,6 +4,7 @@
|
||||
using System;
|
||||
using osu.Framework.Allocation;
|
||||
using osu.Framework.Bindables;
|
||||
using osu.Framework.Graphics;
|
||||
|
||||
namespace osu.Game.Screens.OnlinePlay
|
||||
{
|
||||
@ -11,7 +12,7 @@ namespace osu.Game.Screens.OnlinePlay
|
||||
/// Utility class to track ongoing online operations' progress.
|
||||
/// Can be used to disable interactivity while waiting for a response from online sources.
|
||||
/// </summary>
|
||||
public class OngoingOperationTracker
|
||||
public class OngoingOperationTracker : Component
|
||||
{
|
||||
/// <summary>
|
||||
/// Whether there is an online operation in progress.
|
||||
@ -22,6 +23,11 @@ namespace osu.Game.Screens.OnlinePlay
|
||||
|
||||
private LeasedBindable<bool> leasedInProgress;
|
||||
|
||||
public OngoingOperationTracker()
|
||||
{
|
||||
AlwaysPresent = true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Begins tracking a new online operation.
|
||||
/// </summary>
|
||||
@ -37,7 +43,8 @@ namespace osu.Game.Screens.OnlinePlay
|
||||
leasedInProgress = inProgress.BeginLease(true);
|
||||
leasedInProgress.Value = true;
|
||||
|
||||
return new InvokeOnDisposal(endOperation);
|
||||
// for extra safety, marshal the end of operation back to the update thread if necessary.
|
||||
return new InvokeOnDisposal(() => Scheduler.Add(endOperation, false));
|
||||
}
|
||||
|
||||
private void endOperation()
|
||||
|
@ -54,7 +54,7 @@ namespace osu.Game.Screens.OnlinePlay
|
||||
private readonly Bindable<FilterCriteria> currentFilter = new Bindable<FilterCriteria>(new FilterCriteria());
|
||||
|
||||
[Cached]
|
||||
private readonly OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker();
|
||||
private OngoingOperationTracker ongoingOperationTracker { get; set; }
|
||||
|
||||
[Resolved(CanBeNull = true)]
|
||||
private MusicController music { get; set; }
|
||||
@ -144,7 +144,8 @@ namespace osu.Game.Screens.OnlinePlay
|
||||
};
|
||||
button.Action = () => OpenNewRoom();
|
||||
}),
|
||||
RoomManager = CreateRoomManager()
|
||||
RoomManager = CreateRoomManager(),
|
||||
ongoingOperationTracker = new OngoingOperationTracker()
|
||||
}
|
||||
};
|
||||
|
||||
|
@ -24,7 +24,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
|
||||
public Bindable<FilterCriteria> Filter { get; }
|
||||
|
||||
[Cached]
|
||||
public OngoingOperationTracker OngoingOperationTracker { get; } = new OngoingOperationTracker();
|
||||
public OngoingOperationTracker OngoingOperationTracker { get; }
|
||||
|
||||
protected override Container<Drawable> Content => content;
|
||||
private readonly TestMultiplayerRoomContainer content;
|
||||
@ -39,6 +39,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
|
||||
Client = content.Client;
|
||||
RoomManager = content.RoomManager;
|
||||
Filter = content.Filter;
|
||||
OngoingOperationTracker = content.OngoingOperationTracker;
|
||||
}
|
||||
|
||||
[SetUp]
|
||||
|
@ -25,6 +25,9 @@ namespace osu.Game.Tests.Visual.Multiplayer
|
||||
[Cached]
|
||||
public readonly Bindable<FilterCriteria> Filter = new Bindable<FilterCriteria>(new FilterCriteria());
|
||||
|
||||
[Cached]
|
||||
public readonly OngoingOperationTracker OngoingOperationTracker;
|
||||
|
||||
public TestMultiplayerRoomContainer()
|
||||
{
|
||||
RelativeSizeAxes = Axes.Both;
|
||||
@ -33,6 +36,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
|
||||
{
|
||||
Client = new TestMultiplayerClient(),
|
||||
RoomManager = new TestMultiplayerRoomManager(),
|
||||
OngoingOperationTracker = new OngoingOperationTracker(),
|
||||
content = new Container { RelativeSizeAxes = Axes.Both }
|
||||
});
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user