Skip to content

a - #333

Closed
winnerspiros wants to merge 5 commits into
winnerspiros:masterfrom
ppy:master
Closed

a#333
winnerspiros wants to merge 5 commits into
winnerspiros:masterfrom
ppy:master

Conversation

@winnerspiros

@winnerspiros winnerspiros commented May 12, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Bug Fixes

    • Fixed animation timing to handle negative gameplay rates, preventing incorrect hit-object animations.
    • Improved local score filtering to match rulesets more accurately.
    • Enhanced user rank display to prefer available rank data with sensible fallbacks.
  • Changes

    • Replay playback and spectator mode no longer require the game window to be focused to start gameplay.

Review Change Stack

zihadmahiuddin and others added 4 commits May 12, 2026 13:33
Fixes friend list not showing global rank of the users.
I think #37709 can be closed without any further client-side changes
after `osu-web` is made to return global rank on user searches.
Closes #37715.

The user's database contains several scores in which `ScoreInfo.Ruleset`
is null. How this happened, I'm not sure, it's probably custom rulesets.

The proper way to handle this would be to mark `ScoreInfo.Ruleset` as
nullable and deal with the hundred files of fallout, and also the fact
that `ScoreInfo` is an overloaded mess of a model that is sometimes a
database model and sometimes a post-converted online structure with
things backfilled to fit and I'm just not wanting to waste a week here,
so I'm choosing to look away.

Sidebar: You can't just put a null-propagating operator in the previous
conditional too because analysers will scream that `Ruleset` can't
*possibly* be null! So this uses `RulesetInfo.Equals(RulesetInfo?)`
because that can sorta-kinda handle nulls.
…ive (#37633)

This used to be the case, but recently changed with the introduction of
[pausing when inactive](#37100). The
change was intended to work for local gameplay modes, but it makes less
sense for spectator/replay where you may want to be watching in the
background while doing something else.

Raised via email.
Copilot AI review requested due to automatic review settings May 12, 2026 08:55
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 45008548-185f-4bbd-a0d7-700af705b546

📥 Commits

Reviewing files that changed from the base of the PR and between 7e3cc7c and 6687cc0.

📒 Files selected for processing (1)
  • osu.Game/Screens/Select/PanelLocalRankDisplay.cs

📝 Walkthrough

Walkthrough

Adds a PlayerLoader init flag to optionally bypass the host-active check, normalizes hit animation rate to absolute value, refines local-score subscriptions/filters to use local user and ruleset equality, and falls back to global rank when rendering user rank.

Changes

Gameplay and UI improvements

Layer / File(s) Summary
Window-active gameplay startup configuration
osu.Game/Screens/Play/PlayerLoader.cs, osu.Game/Screens/Play/ReplayPlayerLoader.cs, osu.Game/Screens/Play/SpectatorPlayerLoader.cs
PlayerLoader introduces protected bool WindowShouldBeActiveForGameplayStart { get; init; } = true; and requires host.IsActive.Value in ReadyForGameplay only when this is true. ReplayPlayerLoader and SpectatorPlayerLoader set it to false.
Hit animation rate sign normalization
osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs
UpdateHitStateTransforms now uses Math.Abs(gameplayRate) when deriving animation duration multipliers to avoid negative values.
Local score filtering and subscriptions
osu.Game/Screens/Select/PanelLocalRankDisplay.cs
Adds using osu.Game.Online.API.Requests.Responses, avoids re-subscribing when Beatmap is Equals-identical, introduces a localUser bindable bound from IAPIProvider.LocalUser, and filters local scores by localUser.Value.Id and ruleset.Value.Equals(s.Ruleset).
User rank display fallback
osu.Game/Users/UserPanel.cs
CreateRank() now displays User.Rank?.Rank or falls back to User.Statistics.GlobalRank (or empty string if neither present).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A tiny hop, a careful tweak,
Window checks that makers seek,
Rates made positive, not askew,
Local scores and ranks now true,
Rabbit cheers — small fixes speak!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The pull request title 'a' is vague and non-descriptive, providing no meaningful information about the changeset's content or purpose. Replace the title with a clear, descriptive single sentence that summarizes the main change (e.g., 'Fix gameplay rate calculation and improve player loader window focus handling').
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch master

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gitar-bot

gitar-bot Bot commented May 12, 2026

Copy link
Copy Markdown

Gitar is working

Gitar

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@osu.Game/Users/UserPanel.cs`:
- Line 144: Null reference can occur when reading User.Statistics.GlobalRank in
the Text assignment; update the fallback to null-guard statistics by using
User.Statistics?.GlobalRank (i.e. change the expression using User.Rank?.Rank ??
User.Statistics.GlobalRank to use User.Rank?.Rank ??
User.Statistics?.GlobalRank) so the null-coalescing chain produces null safely
before calling ToLocalisableString on the result in the UserPanel.Text
assignment.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 643746ce-3205-4698-8f3f-3bdb44f4ef35

📥 Commits

Reviewing files that changed from the base of the PR and between 99243f4 and 7e3cc7c.

📒 Files selected for processing (6)
  • osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs
  • osu.Game/Screens/Play/PlayerLoader.cs
  • osu.Game/Screens/Play/ReplayPlayerLoader.cs
  • osu.Game/Screens/Play/SpectatorPlayerLoader.cs
  • osu.Game/Screens/Select/PanelLocalRankDisplay.cs
  • osu.Game/Users/UserPanel.cs


Colour = Colours.BlueLighter,
Text = User.Rank?.Rank?.ToLocalisableString("\\##,##0") ?? string.Empty,
Text = (User.Rank?.Rank ?? User.Statistics.GlobalRank)?.ToLocalisableString("\\##,##0") ?? string.Empty,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Null-guard User.Statistics in the fallback chain.

Line 144 can throw if User.Statistics is null. Please null-check statistics before reading GlobalRank.

Proposed fix
-            Text = (User.Rank?.Rank ?? User.Statistics.GlobalRank)?.ToLocalisableString("\\##,##0") ?? string.Empty,
+            Text = (User.Rank?.Rank ?? User.Statistics?.GlobalRank)?.ToLocalisableString("\\##,##0") ?? string.Empty,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Text = (User.Rank?.Rank ?? User.Statistics.GlobalRank)?.ToLocalisableString("\\##,##0") ?? string.Empty,
Text = (User.Rank?.Rank ?? User.Statistics?.GlobalRank)?.ToLocalisableString("\\##,##0") ?? string.Empty,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@osu.Game/Users/UserPanel.cs` at line 144, Null reference can occur when
reading User.Statistics.GlobalRank in the Text assignment; update the fallback
to null-guard statistics by using User.Statistics?.GlobalRank (i.e. change the
expression using User.Rank?.Rank ?? User.Statistics.GlobalRank to use
User.Rank?.Rank ?? User.Statistics?.GlobalRank) so the null-coalescing chain
produces null safely before calling ToLocalisableString on the result in the
UserPanel.Text assignment.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes a handful of small UI/gameplay-flow correctness tweaks across user/rank displays and gameplay loaders, plus a minor robustness fix in taiko hit animations.

Changes:

  • User panel rank display now falls back to User.Statistics.GlobalRank when User.Rank isn’t available.
  • Local score rank selection now matches rulesets via RulesetInfo.Equals() (shortname-based) rather than manually comparing ShortName.
  • Replay/spectator gameplay loaders no longer wait for the game window to be focused before starting, and taiko hit animation timing now guards against negative gameplay rates.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
osu.Game/Users/UserPanel.cs Adds fallback to statistics global rank for rank text display.
osu.Game/Screens/Select/PanelLocalRankDisplay.cs Uses RulesetInfo.Equals() for ruleset matching when choosing top local score.
osu.Game/Screens/Play/SpectatorPlayerLoader.cs Disables focus requirement before starting gameplay in spectator playback.
osu.Game/Screens/Play/ReplayPlayerLoader.cs Disables focus requirement before starting gameplay in replay playback.
osu.Game/Screens/Play/PlayerLoader.cs Introduces an init-only flag to optionally bypass window-focus gating for gameplay start.
osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs Uses absolute gameplay rate to avoid negative-duration animation issues.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

⚠️ Branch updated during autofix.

The branch was updated while autofix was in progress. Please try again.

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Generating fixes for 1 unresolved review comment...

This may take a few minutes. The auto-fix coding agent will autonomously read files and make edits.

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

Rather than a complete regression from
#37666, the change I made to hide stale
ranks revealed that this subscription was being reinitialised *way* too
often.

Relevant call stack:

<img width="3324" height="1276" alt="2026-05-12 02 15 58@2x"
src="https://github.com/user-attachments/assets/a139a36b-9faa-495a-ab01-bba05cef02d4"
/>

---------

Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants