Skip to content

Commit a4f79f7

Browse files
peppyBartłomiej Dach
andauthored
Apply different workaround to fix ranked play single thread audio issues (ppy#37477)
Thing to make release happen. Reverts ppy#37453 Reverts ppy#37463 Alternative to ppy#37473 Not that I disagree with any of these but I'm just looking to return to what works so we can do a release because we're on a clock here for other reasons. Test which should work but doesn't, so I'm not adding: ```diff diff --git a/osu.Game.Tests/Visual/RankedPlay/TestSceneOpponentPickScreen.cs b/osu.Game.Tests/Visual/RankedPlay/TestSceneOpponentPickScreen.cs index f747004..eb8e360d1e 100644 --- a/osu.Game.Tests/Visual/RankedPlay/TestSceneOpponentPickScreen.cs +++ b/osu.Game.Tests/Visual/RankedPlay/TestSceneOpponentPickScreen.cs @@ -1,12 +1,17 @@ // 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 System.Linq; +using NUnit.Framework; using osu.Framework.Extensions; +using osu.Framework.Testing; using osu.Game.Online.API; using osu.Game.Online.Multiplayer; using osu.Game.Online.Multiplayer.MatchTypes.RankedPlay; using osu.Game.Online.Rooms; using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay; +using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Card; +using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand; namespace osu.Game.Tests.Visual.RankedPlay { @@ -14,6 +19,8 @@ public partial class TestSceneOpponentPickScreen : RankedPlayTestScene { private RankedPlayScreen screen = null!; + private readonly BeatmapRequestHandler requestHandler = new BeatmapRequestHandler(); + public override void SetUpSteps() { base.SetUpSteps(); @@ -26,8 +33,6 @@ public override void SetUpSteps() AddStep("load screen", () => LoadScreen(screen = new RankedPlayScreen(MultiplayerClient.ClientRoom!))); AddUntilStep("screen loaded", () => screen.IsLoaded); - var requestHandler = new BeatmapRequestHandler(); - AddStep("setup request handler", () => ((DummyAPIAccess)API).HandleRequest = requestHandler.HandleRequest); AddStep("set pick state", () => MultiplayerClient.RankedPlayChangeStage(RankedPlayStage.CardPlay, state => state.ActiveUserId = 2).WaitSafely()); @@ -44,7 +49,11 @@ public override void SetUpSteps() }).WaitSafely(); } }); + } + [Test] + public void TestBasic() + { AddWaitStep("wait", 15); AddStep("play beatmap", () => MultiplayerClient.PlayUserCard(2, hand => hand[0]).WaitSafely()); @@ -54,5 +63,29 @@ public override void SetUpSteps() BeatmapID = requestHandler.Beatmaps[0].OnlineID }).WaitSafely()); } + + [Test] + public void TestPickPreviewPlayedOnOpponentPick() + { + RankedPlayCard.SongPreviewContainer? originalPreview = null; + + AddStep("hover first card", + () => InputManager.MoveMouseTo(this.ChildrenOfType<PlayerHandOfCards>().Single().Cards + .First(c => c.Item.PlaylistItem.Value != null && c.Item.PlaylistItem.Value.BeatmapID != requestHandler.Beatmaps[0].OnlineID))); + AddUntilStep("preview playing", () => originalPreview = this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().FirstOrDefault(p => p.IsRunning), () => Is.Not.Null); + + AddStep("play beatmap", () => MultiplayerClient.PlayUserCard(2, hand => hand[0]).WaitSafely()); + AddStep("reveal card", () => MultiplayerClient.RankedPlayRevealUserCard(2, hand => hand[0], new MultiplayerPlaylistItem + { + ID = 0, + BeatmapID = requestHandler.Beatmaps[0].OnlineID + }).WaitSafely()); + + AddUntilStep("wait for original preview stopped", () => originalPreview?.IsRunning, () => Is.False); + + AddUntilStep("preview playing is opponent's pick", + () => ((RankedPlayCard)this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().SingleOrDefault(p => p.IsRunning)?.Parent!).Item.PlaylistItem.Value?.BeatmapID, + () => Is.EqualTo(requestHandler.Beatmaps[0].OnlineID)); + } } } ``` --------- Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
1 parent f1a9be4 commit a4f79f7

10 files changed

Lines changed: 148 additions & 69 deletions

File tree

osu.Game.Tests/Visual/RankedPlay/TestSceneRankedPlayScreen.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
using osu.Game.Graphics.UserInterface;
99
using osu.Game.Online.API;
1010
using osu.Game.Online.API.Requests.Responses;
11+
using osu.Game.Online.Multiplayer;
1112
using osu.Game.Online.Multiplayer.MatchTypes.RankedPlay;
1213
using osu.Game.Online.Rooms;
1314
using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay;
15+
using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Card;
1416
using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand;
1517
using osuTK.Input;
1618

@@ -214,5 +216,41 @@ public void TestHealthChange()
214216
AddWaitStep("wait", 5);
215217
AddStep("change player 2 health", () => MultiplayerClient.RankedPlayChangeUserState(2, state => state.Life = 250_000).WaitSafely());
216218
}
219+
220+
[Test]
221+
public void TestPreviewStopsOnEnteringGameplay()
222+
{
223+
AddStep("join other user", () => MultiplayerClient.AddUser(new APIUser { Id = 2 }));
224+
225+
AddStep("load screen", () => LoadScreen(screen = new RankedPlayScreen(MultiplayerClient.ClientRoom!)));
226+
227+
var requestHandler = new BeatmapRequestHandler();
228+
229+
AddStep("setup request handler", () => ((DummyAPIAccess)API).HandleRequest = requestHandler.HandleRequest);
230+
231+
AddStep("set play phase", () => MultiplayerClient.RankedPlayChangeStage(RankedPlayStage.CardPlay, state => state.ActiveUserId = 1001).WaitSafely());
232+
233+
for (int i = 0; i < 3; i++)
234+
{
235+
int i2 = i;
236+
AddStep("reveal card", () => MultiplayerClient.RankedPlayRevealCard(hand => hand[i2], new MultiplayerPlaylistItem
237+
{
238+
ID = i2,
239+
BeatmapID = requestHandler.Beatmaps[i2].OnlineID
240+
}).WaitSafely());
241+
}
242+
243+
AddStep("hover first card", () => InputManager.MoveMouseTo(this.ChildrenOfType<PlayerHandOfCards>().Single().Cards.First()));
244+
AddUntilStep("preview playing", () => this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().Any(p => p.IsRunning), () => Is.True);
245+
246+
AddWaitStep("wait", 1);
247+
AddStep("play beatmap", () => MultiplayerClient.PlayUserCard(1001, hand => hand[0]).WaitSafely());
248+
249+
AddStep("set warmup", () => MultiplayerClient.RankedPlayChangeStage(RankedPlayStage.GameplayWarmup).WaitSafely());
250+
AddUntilStep("preview running", () => this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().Any(p => p.IsRunning), () => Is.True);
251+
252+
AddStep("load requested", () => ((IMultiplayerClient)MultiplayerClient).LoadRequested());
253+
AddUntilStep("preview stopped", () => this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().Any(p => p.IsRunning), () => Is.False);
254+
}
217255
}
218256
}

osu.Game.Tests/Visual/RankedPlay/TestSceneSongPreview.cs

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Linq;
55
using NUnit.Framework;
6+
using osu.Framework.Bindables;
67
using osu.Framework.Graphics;
78
using osu.Framework.Testing;
89
using osu.Game.Online.API;
@@ -14,9 +15,9 @@ namespace osu.Game.Tests.Visual.RankedPlay
1415
{
1516
public partial class TestSceneSongPreview : RankedPlayTestScene
1617
{
17-
private readonly BeatmapRequestHandler requestHandler = new BeatmapRequestHandler();
18+
private readonly Bindable<bool> previewEnabled = new BindableBool(true);
1819

19-
private PlayerHandOfCards handOfCards = null!;
20+
private readonly BeatmapRequestHandler requestHandler = new BeatmapRequestHandler();
2021

2122
public override void SetUpSteps()
2223
{
@@ -26,6 +27,8 @@ public override void SetUpSteps()
2627

2728
AddStep("add cards", () =>
2829
{
30+
PlayerHandOfCards handOfCards;
31+
2932
Child = handOfCards = new PlayerHandOfCards
3033
{
3134
RelativeSizeAxes = Axes.Both,
@@ -35,36 +38,41 @@ public override void SetUpSteps()
3538
};
3639

3740
foreach (var beatmap in requestHandler.Beatmaps.Take(3))
38-
handOfCards.AddCard(new RevealedRankedPlayCardWithPlaylistItem(beatmap));
41+
{
42+
handOfCards.AddCard(new RevealedRankedPlayCardWithPlaylistItem(beatmap), handCard =>
43+
{
44+
handCard.Card.SongPreviewEnabled.BindTarget = previewEnabled;
45+
});
46+
}
3947
});
4048

41-
AddUntilStep("load tracks", () => this.ChildrenOfType<RankedPlayCard>().All(card => card.SongPreview.TrackLoaded));
49+
AddUntilStep("load tracks", () => this.ChildrenOfType<RankedPlayCard>().All(card => card.PreviewTrackLoaded));
4250
}
4351

4452
[Test]
4553
public void TestSongPreview()
4654
{
4755
AddStep("move mouse to first card", () => InputManager.MoveMouseTo(getCard(0)));
4856

49-
AddAssert("first track running", () => getCard(0).SongPreview.IsRunning);
50-
AddAssert("only one track running", () => this.ChildrenOfType<RankedPlayCard>().Count(c => c.SongPreview.IsRunning) == 1);
57+
AddAssert("first track running", () => getCard(0).PreviewTrackRunning);
58+
AddAssert("only one track running", () => this.ChildrenOfType<RankedPlayCard>().Count(c => c.PreviewTrackRunning) == 1);
5159

5260
AddStep("move mouse to second card", () => InputManager.MoveMouseTo(getCard(1)));
53-
AddAssert("second track running", () => getCard(1).SongPreview.IsRunning);
54-
AddAssert("only one track running", () => this.ChildrenOfType<RankedPlayCard>().Count(c => c.SongPreview.IsRunning) == 1);
5561

56-
AddStep("disable preview", () => handOfCards.CurrentPlayingPreview.Value = null);
57-
AddAssert("no tracks running", () => !this.ChildrenOfType<RankedPlayCard>().Any(c => c.SongPreview.IsRunning));
62+
AddAssert("second track running", () => getCard(1).PreviewTrackRunning);
63+
AddAssert("only one track running", () => this.ChildrenOfType<RankedPlayCard>().Count(c => c.PreviewTrackRunning) == 1);
64+
65+
AddStep("disable preview", () => previewEnabled.Value = false);
66+
67+
AddAssert("no tracks running", () => !this.ChildrenOfType<RankedPlayCard>().Any(c => c.PreviewTrackRunning));
5868

5969
AddStep("move mouse to third card", () => InputManager.MoveMouseTo(getCard(2)));
60-
AddAssert("third track running", () => getCard(2).SongPreview.IsRunning);
6170

62-
AddStep("move mouse away", () => InputManager.MoveMouseTo(Vector2.Zero));
63-
AddAssert("third track running", () => getCard(2).SongPreview.IsRunning);
71+
AddAssert("no tracks running", () => !this.ChildrenOfType<RankedPlayCard>().Any(c => c.PreviewTrackRunning));
6472

65-
AddStep("move mouse to second card", () => InputManager.MoveMouseTo(getCard(1)));
66-
AddAssert("second track running", () => getCard(1).SongPreview.IsRunning);
67-
AddAssert("only one track running", () => this.ChildrenOfType<RankedPlayCard>().Count(c => c.SongPreview.IsRunning) == 1);
73+
AddStep("enable preview", () => previewEnabled.Value = true);
74+
75+
AddAssert("third track running", () => getCard(2).PreviewTrackRunning);
6876
}
6977

7078
private RankedPlayCard getCard(int index) => this.ChildrenOfType<RankedPlayCard>().ElementAt(index);

osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCard.SongPreview.cs

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using osu.Framework.Graphics.Effects;
1414
using osu.Framework.Graphics.Shapes;
1515
using osu.Framework.Graphics.Transforms;
16+
using osu.Framework.Threading;
1617
using osu.Framework.Timing;
1718
using osu.Game.Audio;
1819
using osu.Game.Beatmaps;
@@ -31,6 +32,10 @@ public partial class SongPreviewContainer : Container, IBeatSyncProvider
3132
{
3233
private const double minimum_beat_length = 800;
3334

35+
public readonly Bindable<bool> Enabled = new BindableBool(true);
36+
37+
public readonly Bindable<bool> CardHovered = new BindableBool(true);
38+
3439
public bool TrackLoaded => previewTrack?.TrackLoaded ?? false;
3540

3641
public bool IsRunning => previewTrack?.IsRunning ?? false;
@@ -41,14 +46,14 @@ public partial class SongPreviewContainer : Container, IBeatSyncProvider
4146

4247
private readonly Container overlayLayer;
4348

49+
private bool shouldBePlaying => Enabled.Value && CardHovered.Value;
50+
4451
[Resolved]
4552
private PreviewTrackManager previewTrackManager { get; set; } = null!;
4653

4754
[Resolved]
4855
private OsuColour colours { get; set; } = null!;
4956

50-
public readonly IBindable<SongPreviewContainer?> CurrentPlayingPreview = new Bindable<SongPreviewContainer?>();
51-
5257
public SongPreviewContainer()
5358
{
5459
InternalChildren =
@@ -73,6 +78,33 @@ public SongPreviewContainer()
7378
];
7479
}
7580

81+
protected override void LoadComplete()
82+
{
83+
base.LoadComplete();
84+
85+
Enabled.BindValueChanged(enabled =>
86+
{
87+
if (!enabled.NewValue)
88+
{
89+
stopPreviewIfAvailable();
90+
return;
91+
}
92+
93+
if (shouldBePlaying)
94+
{
95+
startPreviewIfAvailable();
96+
}
97+
});
98+
99+
CardHovered.BindValueChanged(selected =>
100+
{
101+
if (selected.NewValue && shouldBePlaying)
102+
{
103+
startPreviewIfAvailable();
104+
}
105+
});
106+
}
107+
76108
private PreviewTrack? previewTrack;
77109

78110
public void LoadPreview(APIBeatmap beatmap)
@@ -95,30 +127,49 @@ public void LoadPreview(APIBeatmap beatmap)
95127
{
96128
TrackRunning = { BindTarget = trackRunning }
97129
});
130+
131+
if (shouldBePlaying)
132+
startPreviewIfAvailable();
98133
});
99134
}
100135

101-
protected override void Update()
136+
// The following weirdness is a workaround for single-threaded crashes when
137+
// attempting to start a track before it's fully loaded.
138+
//
139+
// See https://github.com/ppy/osu-framework/pull/6727
140+
// https://github.com/ppy/osu/pull/37473
141+
private ScheduledDelegate? trackStartStopAction;
142+
143+
private void startPreviewIfAvailable()
102144
{
103-
base.Update();
145+
if (previewTrack == null)
146+
return;
104147

105-
updatePlayingState();
148+
trackStartStopAction?.Cancel();
149+
150+
if (!previewTrack.TrackLoaded)
151+
{
152+
trackStartStopAction = Schedule(startPreviewIfAvailable);
153+
return;
154+
}
155+
156+
previewTrack?.Start();
106157
}
107158

108-
private void updatePlayingState()
159+
private void stopPreviewIfAvailable()
109160
{
110-
if (previewTrack?.IsLoaded != true)
161+
if (previewTrack == null)
111162
return;
112163

113-
bool shouldBePlaying = CurrentPlayingPreview.Value == this;
164+
trackStartStopAction?.Cancel();
114165

115-
if (shouldBePlaying == previewTrack.IsRunning)
166+
if (!previewTrack.TrackLoaded)
167+
{
168+
trackStartStopAction = Schedule(stopPreviewIfAvailable);
116169
return;
170+
}
117171

118-
if (shouldBePlaying)
119-
previewTrack.Start();
120-
else
121-
previewTrack.Stop();
172+
previewTrack?.Stop();
122173
}
123174

124175
#region IBeatSyncProvider implementation

osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCard.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,29 @@ public partial class RankedPlayCard : CompositeDrawable
3131

3232
private readonly IBindable<MultiplayerPlaylistItem?> playlistItem;
3333

34+
public readonly Bindable<bool> SongPreviewEnabled = new BindableBool(true);
35+
3436
private readonly Container content;
3537
private readonly Container cardContent;
3638
private readonly Container shadow;
3739
private readonly SelectionOutline selectionOutline;
38-
public readonly SongPreviewContainer SongPreview;
40+
private readonly SongPreviewContainer songPreviewContainer;
3941

4042
public bool ShowSelectionOutline
4143
{
4244
set => selectionOutline.FadeTo(value ? 1 : 0, 50);
4345
}
4446

47+
public bool PlayAudioPreview
48+
{
49+
set => songPreviewContainer.CardHovered.Value = value;
50+
}
51+
4552
public float Elevation;
4653

54+
public bool PreviewTrackLoaded => songPreviewContainer.TrackLoaded;
55+
public bool PreviewTrackRunning => songPreviewContainer.IsRunning;
56+
4757
private Sample? cardFlipSample;
4858

4959
[Resolved]
@@ -57,8 +67,9 @@ public RankedPlayCard(RankedPlayCardWithPlaylistItem item)
5767

5868
playlistItem = item.PlaylistItem.GetBoundCopy();
5969

60-
InternalChild = SongPreview = new SongPreviewContainer
70+
InternalChild = songPreviewContainer = new SongPreviewContainer
6171
{
72+
Enabled = { BindTarget = SongPreviewEnabled },
6273
RelativeSizeAxes = Axes.Both,
6374
Anchor = Anchor.Centre,
6475
Origin = Anchor.Centre,
@@ -162,7 +173,7 @@ private void loadCardContentAsync(MultiplayerPlaylistItem playlistItem) => Task.
162173
Schedule(() =>
163174
{
164175
SetContent(new RankedPlayCardContent(beatmap));
165-
SongPreview.LoadPreview(beatmap);
176+
songPreviewContainer.LoadPreview(beatmap);
166177
});
167178
});
168179

osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/DiscardScreen.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,14 +253,13 @@ private void playDiscardAnimation()
253253

254254
foreach (var item in discardedCards)
255255
{
256-
if (!playerHand.DetachCard(item, out var card, out Quad drawQuad))
256+
if (!playerHand.RemoveCard(item, out var card, out Quad drawQuad))
257257
return;
258258

259259
card.Anchor = Anchor.Centre;
260260
card.Origin = Anchor.Centre;
261261

262-
if (playerHand.CurrentPlayingPreview.Value == card.SongPreview)
263-
playerHand.CurrentPlayingPreview.Value = null;
262+
card.SongPreviewEnabled.Value = false;
264263

265264
card.MatchScreenSpaceDrawQuad(drawQuad, CenterRow);
266265

@@ -334,7 +333,7 @@ private void presentRemainingCards()
334333

335334
foreach (var item in matchInfo.PlayerCards)
336335
{
337-
if (playerHand.DetachCard(item, out var card, out Quad drawQuad))
336+
if (playerHand.RemoveCard(item, out var card, out Quad drawQuad))
338337
{
339338
card.MatchScreenSpaceDrawQuad(drawQuad, CenterRow);
340339

osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.HandCard.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ protected virtual void OnStateChanged(ValueChangedEvent<RankedPlayCardState> sta
104104
handOfCards.OnCardStateChanged(this, state);
105105

106106
Card.ShowSelectionOutline = state.NewValue.Selected;
107+
Card.PlayAudioPreview = state.NewValue.Hovered;
107108

108109
switch (state.NewValue.Pressed, state.OldValue.Pressed)
109110
{

osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public void Contract()
113113

114114
public void AddCard(RankedPlayCardWithPlaylistItem item, Action<HandCard>? setupAction = null) => AddCard(new RankedPlayCard(item), setupAction);
115115

116-
public virtual void AddCard(RankedPlayCard card, Action<HandCard>? setupAction = null)
116+
public void AddCard(RankedPlayCard card, Action<HandCard>? setupAction = null)
117117
{
118118
if (cardLookup.ContainsKey(card.Item.Card))
119119
return;
@@ -159,7 +159,7 @@ public bool RemoveCard(RankedPlayCardWithPlaylistItem item)
159159
/// <param name="card">Contained <see cref="RankedPlayCard"/></param>
160160
/// <param name="screenSpaceDrawQuad"><see cref="Drawable.ScreenSpaceDrawQuad"/> of the removed card</param>
161161
/// <returns>Whether a card was found for the provided item</returns>
162-
public virtual bool DetachCard(RankedPlayCardWithPlaylistItem item, [MaybeNullWhen(false)] out RankedPlayCard card, out Quad screenSpaceDrawQuad)
162+
public bool RemoveCard(RankedPlayCardWithPlaylistItem item, [MaybeNullWhen(false)] out RankedPlayCard card, out Quad screenSpaceDrawQuad)
163163
{
164164
if (!cardLookup.Remove(item.Card, out var drawable))
165165
{

0 commit comments

Comments
 (0)