Skip to content

Commit

Permalink
Merge pull request #20178 from smoogipoo/fix-match-creation-beatmap-s…
Browse files Browse the repository at this point in the history 
…elect

Fix several weird scenarios with online play song selection
  • Loading branch information
@peppy
peppy committed Sep 8, 2022
2 parents f0fdfb1 + a90ca94 commit 6945c43
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 138 deletions.
36 changes: 2 additions & 34 deletions osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using osu.Game.Online.Rooms;
using osu.Game.Overlays.Mods;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Catch;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Osu;
using osu.Game.Rulesets.Osu.Mods;
Expand Down Expand Up @@ -68,37 +67,6 @@ public override void SetUpSteps()
AddUntilStep("wait for present", () => songSelect.IsCurrentScreen() && songSelect.BeatmapSetsLoaded);
}

[Test]
public void TestBeatmapRevertedOnExitIfNoSelection()
{
BeatmapInfo selectedBeatmap = null;

AddStep("select beatmap",
() => songSelect.Carousel.SelectBeatmap(selectedBeatmap = beatmaps.Where(beatmap => beatmap.Ruleset.OnlineID == new OsuRuleset().LegacyID).ElementAt(1)));
AddUntilStep("wait for selection", () => Beatmap.Value.BeatmapInfo.Equals(selectedBeatmap));

AddStep("exit song select", () => songSelect.Exit());
AddAssert("beatmap reverted", () => Beatmap.IsDefault);
}

[Test]
public void TestModsRevertedOnExitIfNoSelection()
{
AddStep("change mods", () => SelectedMods.Value = new[] { new OsuModDoubleTime() });

AddStep("exit song select", () => songSelect.Exit());
AddAssert("mods reverted", () => SelectedMods.Value.Count == 0);
}

[Test]
public void TestRulesetRevertedOnExitIfNoSelection()
{
AddStep("change ruleset", () => Ruleset.Value = new CatchRuleset().RulesetInfo);

AddStep("exit song select", () => songSelect.Exit());
AddAssert("ruleset reverted", () => Ruleset.Value.Equals(new OsuRuleset().RulesetInfo));
}

[Test]
public void TestBeatmapConfirmed()
{
Expand Down Expand Up @@ -152,8 +120,8 @@ private class TestMultiplayerMatchSongSelect : MultiplayerMatchSongSelect

public new BeatmapCarousel Carousel => base.Carousel;

public TestMultiplayerMatchSongSelect(Room room, WorkingBeatmap beatmap = null, RulesetInfo ruleset = null)
: base(room, null, beatmap, ruleset)
public TestMultiplayerMatchSongSelect(Room room)
: base(room)
{
}
}
Expand Down
3 changes: 3 additions & 0 deletions osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,9 @@ private void selectedItemChanged()

private void updateWorkingBeatmap()
{
if (SelectedItem.Value == null || !this.IsCurrentScreen())
return;

var beatmap = SelectedItem.Value?.Beatmap;

// Retrieve the corresponding local beatmap, since we can't directly use the playlist's beatmap info
Expand Down
51 changes: 23 additions & 28 deletions osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
// 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.ComponentModel;
using System.Diagnostics;
using JetBrains.Annotations;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Extensions;
Expand All @@ -30,12 +27,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match
{
public class MultiplayerMatchSettingsOverlay : RoomSettingsOverlay
{
private MatchSettings settings;
private MatchSettings settings = null!;

protected override OsuButton SubmitButton => settings.ApplyButton;

[Resolved]
private OngoingOperationTracker ongoingOperationTracker { get; set; }
private OngoingOperationTracker ongoingOperationTracker { get; set; } = null!;

protected override bool IsLoading => ongoingOperationTracker.InProgress.Value;

Expand All @@ -57,20 +54,21 @@ protected class MatchSettings : OnlinePlayComposite
{
private const float disabled_alpha = 0.2f;

public Action SettingsApplied;
public Action? SettingsApplied;

public OsuTextBox NameField, MaxParticipantsField;
public MatchTypePicker TypePicker;
public OsuEnumDropdown<QueueMode> QueueModeDropdown;
public OsuTextBox PasswordTextBox;
public OsuCheckbox AutoSkipCheckbox;
public TriangleButton ApplyButton;
public OsuTextBox NameField = null!;
public OsuTextBox MaxParticipantsField = null!;
public MatchTypePicker TypePicker = null!;
public OsuEnumDropdown<QueueMode> QueueModeDropdown = null!;
public OsuTextBox PasswordTextBox = null!;
public OsuCheckbox AutoSkipCheckbox = null!;
public TriangleButton ApplyButton = null!;

public OsuSpriteText ErrorText;
public OsuSpriteText ErrorText = null!;

private OsuEnumDropdown<StartMode> startModeDropdown;
private OsuSpriteText typeLabel;
private LoadingLayer loadingLayer;
private OsuEnumDropdown<StartMode> startModeDropdown = null!;
private OsuSpriteText typeLabel = null!;
private LoadingLayer loadingLayer = null!;

public void SelectBeatmap()
{
Expand All @@ -79,26 +77,23 @@ public void SelectBeatmap()
}

[Resolved]
private MultiplayerMatchSubScreen matchSubScreen { get; set; }
private MultiplayerMatchSubScreen matchSubScreen { get; set; } = null!;

[Resolved]
private IRoomManager manager { get; set; }
private IRoomManager manager { get; set; } = null!;

[Resolved]
private MultiplayerClient client { get; set; }
private MultiplayerClient client { get; set; } = null!;

[Resolved]
private OngoingOperationTracker ongoingOperationTracker { get; set; }
private OngoingOperationTracker ongoingOperationTracker { get; set; } = null!;

private readonly IBindable<bool> operationInProgress = new BindableBool();

[CanBeNull]
private IDisposable applyingSettingsOperation;

private readonly Room room;

private Drawable playlistContainer;
private DrawableRoomPlaylist drawablePlaylist;
private IDisposable? applyingSettingsOperation;
private Drawable playlistContainer = null!;
private DrawableRoomPlaylist drawablePlaylist = null!;

public MatchSettings(Room room)
{
Expand Down Expand Up @@ -423,7 +418,7 @@ private void apply()
else
room.MaxParticipants.Value = null;

manager?.CreateRoom(room, onSuccess, onError);
manager.CreateRoom(room, onSuccess, onError);
}
}

Expand Down Expand Up @@ -466,7 +461,7 @@ private void onError(string text)
public class CreateOrUpdateButton : TriangleButton
{
[Resolved(typeof(Room), nameof(Room.RoomID))]
private Bindable<long?> roomId { get; set; }
private Bindable<long?> roomId { get; set; } = null!;

protected override void LoadComplete()
{
Expand Down
23 changes: 5 additions & 18 deletions osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSongSelect.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
using osu.Framework.Bindables;
using osu.Framework.Logging;
using osu.Framework.Screens;
using osu.Game.Beatmaps;
using osu.Game.Graphics.UserInterface;
using osu.Game.Online.Multiplayer;
using osu.Game.Online.Rooms;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Mods;
using osu.Game.Screens.Select;

Expand All @@ -27,7 +25,7 @@ public class MultiplayerMatchSongSelect : OnlinePlaySongSelect
private OngoingOperationTracker operationTracker { get; set; } = null!;

private readonly IBindable<bool> operationInProgress = new Bindable<bool>();
private readonly long? itemToEdit;
private readonly PlaylistItem? itemToEdit;

private LoadingLayer loadingLayer = null!;
private IDisposable? selectionOperation;
Expand All @@ -37,21 +35,10 @@ public class MultiplayerMatchSongSelect : OnlinePlaySongSelect
/// </summary>
/// <param name="room">The room.</param>
/// <param name="itemToEdit">The item to be edited. May be null, in which case a new item will be added to the playlist.</param>
/// <param name="beatmap">An optional initial beatmap selection to perform.</param>
/// <param name="ruleset">An optional initial ruleset selection to perform.</param>
public MultiplayerMatchSongSelect(Room room, long? itemToEdit = null, WorkingBeatmap? beatmap = null, RulesetInfo? ruleset = null)
: base(room)
public MultiplayerMatchSongSelect(Room room, PlaylistItem? itemToEdit = null)
: base(room, itemToEdit)
{
this.itemToEdit = itemToEdit;

if (beatmap != null || ruleset != null)
{
Schedule(() =>
{
if (beatmap != null) Beatmap.Value = beatmap;
if (ruleset != null) Ruleset.Value = ruleset;
});
}
}

[BackgroundDependencyLoader]
Expand Down Expand Up @@ -80,7 +67,7 @@ protected override bool SelectItem(PlaylistItem item)
{
if (operationInProgress.Value)
{
Logger.Log($"{nameof(SelectedItem)} aborted due to {nameof(operationInProgress)}");
Logger.Log($"{nameof(SelectItem)} aborted due to {nameof(operationInProgress)}");
return false;
}

Expand All @@ -92,7 +79,7 @@ protected override bool SelectItem(PlaylistItem item)

var multiplayerItem = new MultiplayerPlaylistItem
{
ID = itemToEdit ?? 0,
ID = itemToEdit?.ID ?? 0,
BeatmapID = item.Beatmap.OnlineID,
BeatmapChecksum = item.Beatmap.MD5Hash,
RulesetID = item.RulesetID,
Expand Down
14 changes: 2 additions & 12 deletions osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ public class MultiplayerMatchSubScreen : RoomSubScreen, IHandlePresentBeatmap
[Resolved]
private MultiplayerClient client { get; set; }

[Resolved]
private BeatmapManager beatmapManager { get; set; }

private readonly IBindable<bool> isConnected = new Bindable<bool>();

private AddItemButton addItemButton;

public MultiplayerMatchSubScreen(Room room)
Expand Down Expand Up @@ -227,12 +222,7 @@ internal void OpenSongSelection(PlaylistItem itemToEdit = null)
if (!this.IsCurrentScreen())
return;

int id = itemToEdit?.Beatmap.OnlineID ?? Room.Playlist.Last().Beatmap.OnlineID;
var localBeatmap = beatmapManager.QueryBeatmap(b => b.OnlineID == id);

var workingBeatmap = localBeatmap == null ? null : beatmapManager.GetWorkingBeatmap(localBeatmap);

this.Push(new MultiplayerMatchSongSelect(Room, itemToEdit?.ID, workingBeatmap));
this.Push(new MultiplayerMatchSongSelect(Room, itemToEdit));
}

protected override Drawable CreateFooter() => new MultiplayerMatchFooter();
Expand Down Expand Up @@ -424,7 +414,7 @@ public void PresentBeatmap(WorkingBeatmap beatmap, RulesetInfo ruleset)
return;
}

this.Push(new MultiplayerMatchSongSelect(Room, client.Room.Settings.PlaylistItemId, beatmap, ruleset));
this.Push(new MultiplayerMatchSongSelect(Room, Room.Playlist.Single(item => item.ID == client.Room.Settings.PlaylistItemId)));
}

protected override void Dispose(bool isDisposing)
Expand Down
Loading

0 comments on commit 6945c43

Please sign in to comment.