a - #217
Merged
Merged
Conversation
Best that can be done for the time being.
https://discord.com/channels/188630481301012481/1440912440224120882/1495066077175222422 Haven't really tested whether this works - dev server is also lacking data in general, however relying on the previous example set by quick play: https://github.com/ppy/osu/blob/1168c477d95f019aea2b676ef51db4ab6cd7253f/osu.Game/Screens/OnlinePlay/Matchmaking/Match/BeatmapSelect/MatchmakingSelectPanel.CardContentBeatmap.cs#L418-L421
Previously the current stage overlay would not display the decimal part of the damage multiplier. Resolves #37357.
Feedback that this can be annoying when waiting for a match with the game in the background for several minutes. Agree with this.
People doing stupid shit submitting stupid errors we don't care about.
Closes: #37402 Issue was `HandOfCards.CardContainer.Compare` working under the assumption that the it would never be called with the same child for both entries, which can happen when doing a `BinarySearch` (called in `RemoveInternal`). This lead to `IndexOf` returning a negative value despite the card being present in the container, and the drawable getting disposed but not actually removed. I also included a precautionary `cardContainer.Sort()` call before the removal as well, seemed to work without that from testing but better not rely on that. TLDR: card container child sorting was unstable due to poor assumptions
Exposed to be running by 48434dd which caused test failures. You'd think that the `IsDeployedBuild` check would catch it but it doesn't. `IsDeployedBuild` is `AssemblyVersion.Major > 0`, and the assembly version is taken from the entry assembly. In tests the entry assembly is either resharper or nunit.
Closes #37345 Aside from 9f4b1c8, this also tweaks the grid sizings on mobile which were the secondary cause of the android crashes. I'm bothering because it looked awful even with the primary crash fixed. Only tested on android. I wanted to test visually on iOS but rider doesn't want to talk to xcode *or* my iPhone anymore, so I give up. | before | after | | :-: | :-: | | <img width="2400" height="1080" alt="Screenshot_2026-04-20-12-04-17-44_25a1a32208bbcdc1d450b7aa854bb161" src="https://github.com/user-attachments/assets/e9bac850-7256-4a0d-8358-992490e1381d" /> | <img width="2400" height="1080" alt="Screenshot_2026-04-20-12-07-35-26_25a1a32208bbcdc1d450b7aa854bb161" src="https://github.com/user-attachments/assets/a525485e-f2b5-4252-ae50-dc84c29945ec" /> |
Intends to close #37408. I have got to say, the way ranked play apparently re-invents screen sub-stacks *again* in a slightly different way to everything else before had me *very* confused as to why things I would expect to get called aren't getting called.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis pull request introduces audio handling improvements, test enhancements, and targeted bug fixes across the osu game codebase. Changes include converting audio samples to drawable components, adding guards for division operations, modifying card removal logic, extending test coverage, and adjusting UI responsiveness for mobile platforms. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adjusts several matchmaking / ranked-play UI and audio behaviours, adds a couple of safety fixes, and updates/extends visual tests.
Changes:
- Add exit-time audio silencing/stopping hooks for ranked-play results and queue waiting loop, plus a manual visual test for results audio.
- Fix card-hand removal ordering edge case during dragging, tweak stage overlay multiplier formatting, and adjust queue layout for mobile.
- Guard against division-by-zero in rating distribution graph line sizing.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| osu.Game/Utils/SentryLogger.cs | Skips Sentry initialisation under NUnit; modifies Sentry DSN initialisation. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/ResultsScreen.cs | Tracks the loaded results MainPanel and stops/mutes samples on exit. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/ResultsScreen.MainPanel.cs | Refactors samples to DrawableSample under an AudioContainer; adds StopAllSamples(). |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/RankedPlayStageOverlay.cs | Formats multiplier text via standard formatting (1 decimal). |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/RankedPlayScreen.cs | Sets lobby activity and ensures subscreen exit hook is called during screen exit. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs | Forces a sort before removal to avoid binary-search/remove issues during per-frame reordering. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCardContent.cs | Updates context menu action to fetch/show beatmap by online ID. |
| osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs | Moves waiting-loop to DrawableSample, adds fade-out behaviour, and adjusts layout sizing on mobile. |
| osu.Game/Screens/OnlinePlay/Matchmaking/Queue/RatingDistributionGraph.cs | Adds zero-size guards to prevent divide-by-zero when computing 1px line thickness. |
| osu.Game.Tests/Visual/RankedPlay/TestSceneResultsScreen.cs | Adds an explicit manual test for verifying results audio stops on exit. |
| osu.Game.Tests/Visual/RankedPlay/TestSceneRankedPlayStageOverlay.cs | Adds a multiplier slider to the stage overlay visual test. |
| osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs | Adds a test covering removal while dragging. |
Comments suppressed due to low confidence (1)
osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs:663
stopWaitingLoopPlayback()disposeswaitingLoopChannelbut never clears the field. BecausestopWaitingLoopPlayback()is called from multiple places (state transitions, start, Dispose), this can lead to repeated Stop/Dispose calls on the same channel instance. SetwaitingLoopChannel = nullafter disposing (and consider resettingwaitingLoopvolume state) so subsequent calls are idempotent and safe.
private void stopWaitingLoopPlayback()
{
waitingLoopChannel?.Stop();
waitingLoopChannel?.Dispose();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+570
to
+572
| sampleContainer.Volume.Value = 0; | ||
| playerScoreTickChannel?.Stop(); | ||
| opponentScoreTickChannel?.Stop(); |
Comment on lines
59
to
62
| sentrySession = SentrySdk.Init(options => | ||
| { | ||
| options.Dsn = "https://ad9f78529cef40ac874afb95a9aca04e@sentry.ppy.sh/2"; | ||
| options.Dsn = "https://localhost"; | ||
| options.AutoSessionTracking = true; |
Comment on lines
643
to
657
| private void startWaitingLoopPlayback() | ||
| { | ||
| stopWaitingLoopPlayback(); | ||
|
|
||
| waitingLoopChannel = waitingLoopSample?.GetChannel(); | ||
| waitingLoopChannel = waitingLoop.GetChannel(); | ||
| if (waitingLoopChannel == null) | ||
| return; | ||
|
|
||
| waitingLoopChannel.Looping = true; | ||
| waitingLoopChannel?.Play(); | ||
|
|
||
| waitingLoop.VolumeTo(1) | ||
| .Delay(2000) | ||
| .VolumeTo(0, 12000); | ||
| } |
Comment on lines
+525
to
527
| if (Precision.DefinitelyBigger(Parent!.ScreenSpaceDrawQuad.Width, 0)) | ||
| Width = Parent.DrawWidth / Parent.ScreenSpaceDrawQuad.Width; | ||
| } |
| base.Update(); | ||
| Height = Parent!.DrawHeight / Parent.ScreenSpaceDrawQuad.Height; | ||
| if (Precision.DefinitelyBigger(Parent!.ScreenSpaceDrawQuad.Height, 0)) | ||
| Height = Parent!.DrawHeight / Parent.ScreenSpaceDrawQuad.Height; |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Gitar
StopAllSamples()inRankedResultsScreento ensure audio stops immediately when exiting the screen.This will update automatically on new commits.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Refactor
Chores