Merge ppy/osu upstream (9 commits) - #340
Merged
Merged
Conversation
It looks to be attracting AI wangs.
- Added a small breakdown animation to the results screen. - Added individual multiplier text to user corner pieces. - Removed global multiplier text from the stage overlay, since we're going with individual multipliers. https://github.com/user-attachments/assets/47cec478-6ad5-49fa-9f69-b6df079ce41c (This is dev design and I'm focusing on functionality rather than presentation for now.) The implementation might be over-engineered a bit, but I'm not sure on the final structure of things and I want to give a bit of elasticity to the system, so I've frankensteined a new "damage sources" list inside `RankedPlayDamageInfo` that the results screen uses to display the breakdown. If the server doesn't provide a breakdown (e.g. by client and server being slightly out-of-date), the results screen will behave as it does on current `master`. In other words this is forwards/backwards compatible. --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
We already [did this for quick play](ppy#36025) but it was never carried across. Some of the new UI *could* work with UI scale with more consideration, but for now, let's disable it globally to fix cases like the results screen which people completely unusable at higher scales. --- @ppy/team-client would hope to get this into today's build, if priority review could be given to it. Closes ppy#37407. - Note that this adds a UI scale slider to all `ScreenTestScenes`. In some testing, this seems to work just fine. - May be worth reading 6c8dd58 commit message for one future concern i have.
Addresses ppy#37777. Note: please don't merge until after the imminent release. Would rather not add new features at this point.
https://github.com/user-attachments/assets/2a511e0d-51f8-4abf-a3ab-de0992618b6b This implements a rather opinionated UX that's designed to be a middle ground between the previous lazer behaviour of unconditionally inheriting the last slider's velocity and just a textbox that you'd need to manually fiddle with every time. As to what that means, precisely: - By default, the control follows the last slider's velocity (updates on seeks as well as changes to existing objects). - When the slider control in the toolbox is manually adjusted, the control decouples from the last slider's velocity and instead uses the last manually-specified value. - There is a button that allows the user to couple back to the last slider's velocity if they consider to have made a mistake in adjusting it. - Upon successful placement of a slider, the control reverts to following the last slider's velocity. Of note, this control *only interacts with and affects the next placed slider*. It is in no way coupled to any selected objects. This may be confusing to users but was an intentional choice to limit complexity (what if there are multiple selected objects with multiple velocities?) For adjusting existing objects you can use the green pieces on the timeline, which notably do support changing multiple selected objects at once. --- - Closes ppy#36844 - Supersedes / closes ppy#33707
- Part of ppy#37818 - Continued from ppy#37355 (comment) ## Overview This PR introduces an alternative API, `ScoreMultiplierCalculator`, to be used going forward for calculating mod multipliers. The reason for introducing this new API is that it has been requested that: - For any two given mods, it should be possible to have the combined mod multipliers of them in combination be *different* than the product of the individual mods' multipliers in isolation, i.e. $mult( \\{ A, B \\} ) \neq mult( \\{ A \\} ) \cdot mult( \\{ B \\} )$. - For an individual mod, it should be possible to have the mod multipliers depend on a quantity that is *not* the presence of another mod or the direct value of a setting on the mod. This capability is being demonstrated in this PR via the `osu.Game.Tests.Rulesets.Scoring.ScoreMultiplierCalculatorTest` test fixture. ## Parity with `Mod.ScoreMultiplier` This PR contains a `ScoreMultiplierCalculator` implementation for each of the built-in four rulesets. The abstract `osu.Game.Tests.Rulesets.RulesetScoreMultiplierTest` and its four derived ruleset-specific test fixtures were written to ensure that the new implementations do not diverge from the current state of affairs. `Mod.ScoreMultiplier` is not removed in this diff to keep size low. It will be removed as a follow-up. ## Performance This PR contains a benchmark comparing the current implementation via `Mod.ScoreMultiplier` and the new `ScoreMultiplierCalculator` API. Results below. <details> | Method | Times | Mods | Mean | Error | StdDev | Gen0 | Allocated | |---------------------- |------ |--------------------- |--------------:|------------:|------------:|--------:|----------:| | ViaModScoreMultiplier | 1 | mods (...)tings [27] | 121.171 ns | 1.5284 ns | 1.4297 ns | 0.0782 | 656 B | | ViaCalculator | 1 | mods (...)tings [27] | 248.509 ns | 1.9313 ns | 1.6127 ns | 0.1364 | 1144 B | | ViaModScoreMultiplier | 1 | multiple mods | 128.357 ns | 0.4282 ns | 0.4006 ns | 0.0782 | 656 B | | ViaCalculator | 1 | multiple mods | 252.953 ns | 1.2860 ns | 1.2029 ns | 0.1364 | 1144 B | | ViaModScoreMultiplier | 1 | no mods | 3.007 ns | 0.0345 ns | 0.0288 ns | - | - | | ViaCalculator | 1 | no mods | 14.802 ns | 0.0616 ns | 0.0576 ns | 0.0134 | 112 B | | ViaModScoreMultiplier | 1 | single mod | 40.271 ns | 0.1238 ns | 0.1098 ns | 0.0258 | 216 B | | ViaCalculator | 1 | single mod | 113.033 ns | 0.3140 ns | 0.2937 ns | 0.0842 | 704 B | | ViaModScoreMultiplier | 1 | single mod 2 | 3.653 ns | 0.0384 ns | 0.0359 ns | 0.0038 | 32 B | | ViaCalculator | 1 | single mod 2 | 78.172 ns | 0.0680 ns | 0.0603 ns | 0.0621 | 520 B | | ViaModScoreMultiplier | 10 | mods (...)tings [27] | 1,169.609 ns | 4.3058 ns | 4.0276 ns | 0.7839 | 6560 B | | ViaCalculator | 10 | mods (...)tings [27] | 2,575.264 ns | 21.2705 ns | 19.8964 ns | 1.3657 | 11440 B | | ViaModScoreMultiplier | 10 | multiple mods | 1,171.775 ns | 6.2332 ns | 5.2050 ns | 0.7839 | 6560 B | | ViaCalculator | 10 | multiple mods | 2,579.593 ns | 22.1010 ns | 20.6733 ns | 1.3657 | 11440 B | | ViaModScoreMultiplier | 10 | no mods | 35.943 ns | 0.1665 ns | 0.1476 ns | - | - | | ViaCalculator | 10 | no mods | 154.980 ns | 0.2381 ns | 0.1988 ns | 0.1338 | 1120 B | | ViaModScoreMultiplier | 10 | single mod | 404.185 ns | 1.3190 ns | 1.2338 ns | 0.2580 | 2160 B | | ViaCalculator | 10 | single mod | 1,167.279 ns | 6.1641 ns | 5.7659 ns | 0.8411 | 7040 B | | ViaModScoreMultiplier | 10 | single mod 2 | 42.128 ns | 0.2878 ns | 0.2692 ns | 0.0382 | 320 B | | ViaCalculator | 10 | single mod 2 | 775.435 ns | 2.3318 ns | 2.1811 ns | 0.6208 | 5200 B | | ViaModScoreMultiplier | 100 | mods (...)tings [27] | 11,623.346 ns | 51.7174 ns | 43.1863 ns | 7.8430 | 65600 B | | ViaCalculator | 100 | mods (...)tings [27] | 25,252.987 ns | 44.4352 ns | 39.3906 ns | 13.6719 | 114400 B | | ViaModScoreMultiplier | 100 | multiple mods | 11,928.536 ns | 35.2079 ns | 32.9334 ns | 7.8430 | 65600 B | | ViaCalculator | 100 | multiple mods | 25,399.378 ns | 152.4597 ns | 127.3108 ns | 13.6719 | 114400 B | | ViaModScoreMultiplier | 100 | no mods | 328.158 ns | 0.5827 ns | 0.5165 ns | - | - | | ViaCalculator | 100 | no mods | 1,517.485 ns | 10.2304 ns | 9.5695 ns | 1.3390 | 11200 B | | ViaModScoreMultiplier | 100 | single mod | 3,986.251 ns | 24.2523 ns | 21.4991 ns | 2.5787 | 21600 B | | ViaCalculator | 100 | single mod | 11,479.514 ns | 23.3738 ns | 20.7203 ns | 8.4076 | 70400 B | | ViaModScoreMultiplier | 100 | single mod 2 | 385.679 ns | 3.5190 ns | 3.2917 ns | 0.3824 | 3200 B | | ViaCalculator | 100 | single mod 2 | 7,658.646 ns | 21.8274 ns | 19.3494 ns | 6.2103 | 52000 B | </details> While the calculator is obviously slower, in my view it is not egregiously so. The main overheads both time- and memory-wise are collection allocations for the dictionary and the set which I consider to be directly caused by the requested additional complexity and as such I don't really consider them eliminable. I have tried and applied some micro-optimisations (e2469ce, cb33abe), albeit with negligible effect. I have also tried to key the mods by `Acronym` instead of by `Type` and the difference was basically nil. <details> <summary>patch for keying by acronym</summary> ```diff diff --git a/osu.Game/Rulesets/Scoring/ScoreMultiplierCalculator.cs b/osu.Game/Rulesets/Scoring/ScoreMultiplierCalculator.cs index 772f9d1..7f5907cbda 100644 --- a/osu.Game/Rulesets/Scoring/ScoreMultiplierCalculator.cs +++ b/osu.Game/Rulesets/Scoring/ScoreMultiplierCalculator.cs @@ -13,26 +13,26 @@ namespace osu.Game.Rulesets.Scoring /// </summary> public class ScoreMultiplierCalculator { - private static readonly List<(Type[] mods, Func<Mod[], double> multiplier)> combination_multipliers = []; - private static readonly Dictionary<Type, Func<Mod, ScoreMultiplierCalculator, double>> single_multipliers_with_context = []; - private static readonly Dictionary<Type, Func<Mod, double>> single_multipliers = []; + private static readonly List<(string[] modAcronyms, Func<Mod[], double> multiplier)> combination_multipliers = []; + private static readonly Dictionary<string, Func<Mod, ScoreMultiplierCalculator, double>> single_multipliers_with_context = []; + private static readonly Dictionary<string, Func<Mod, double>> single_multipliers = []; /// <summary> /// Defines a flat, setting-independent score multiplier for the given <typeparamref name="TMod"/>. /// </summary> public static void Single<TMod>(double hasMultiplier) - where TMod : Mod + where TMod : Mod, new() { - single_multipliers[typeof(TMod)] = _ => hasMultiplier; + single_multipliers[new TMod().Acronym] = _ => hasMultiplier; } /// <summary> /// Defines a setting-dependent score multiplier for the given <typeparamref name="TMod"/>. /// </summary> public static void Single<TMod>(Func<TMod, double> hasMultiplier) - where TMod : Mod + where TMod : Mod, new() { - single_multipliers[typeof(TMod)] = mod => hasMultiplier.Invoke((TMod)mod); + single_multipliers[new TMod().Acronym] = mod => hasMultiplier.Invoke((TMod)mod); } /// <summary> @@ -40,20 +40,20 @@ public static void Single<TMod>(Func<TMod, double> hasMultiplier) /// The multiplier calculation is given additional context to calculate the multiplier via the <typeparamref name="TContext"/> type instance. /// </summary> public static void Single<TMod, TContext>(Func<TMod, TContext, double> hasMultiplier) - where TMod : Mod + where TMod : Mod, new() where TContext : ScoreMultiplierCalculator { - single_multipliers_with_context[typeof(TMod)] = (mod, context) => hasMultiplier.Invoke((TMod)mod, (TContext)context); + single_multipliers_with_context[new TMod().Acronym] = (mod, context) => hasMultiplier.Invoke((TMod)mod, (TContext)context); } /// <summary> /// Defines a score multiplier specific to when both <typeparamref name="T1"/> and <typeparamref name="T2"/> mods are present. /// </summary> public static void Combination<T1, T2>(Func<T1, T2, double> hasMultiplier) - where T1 : Mod - where T2 : Mod + where T1 : Mod, new() + where T2 : Mod, new() { - combination_multipliers.Add(([typeof(T1), typeof(T2)], mods => hasMultiplier((T1)mods[0], (T2)mods[1]))); + combination_multipliers.Add(([new T1().Acronym, new T2().Acronym], mods => hasMultiplier((T1)mods[0], (T2)mods[1]))); } /// <summary> @@ -61,7 +61,7 @@ public static void Combination<T1, T2>(Func<T1, T2, double> hasMultiplier) /// </summary> public double CalculateFor(IEnumerable<Mod> mods) { - var allModsByType = mods.ToDictionary(m => m.GetType()); + var allModsByType = mods.ToDictionary(m => m.Acronym); if (allModsByType.Count == 0) return 1; @@ -83,7 +83,7 @@ public double CalculateFor(IEnumerable<Mod> mods) } } - foreach (var modType in remainingModTypes) + foreach (string modType in remainingModTypes) { if (single_multipliers.TryGetValue(modType, out var multiplier)) result *= multiplier(allModsByType[modType]); ``` </details> One particular parallel thread that may warrant follow-up is that `Mod.UsesDefaultConfiguration` is disproportionately expensive due to calling into regexes via Humanizer internals. <img width="1517" height="517" alt="Screenshot_2026-05-19_at_10 58 30" src="https://github.com/user-attachments/assets/68309a8c-74e7-4f96-8ef9-62868eeca337" />
…-into-ours Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
winnerspiros
May 19, 2026 13:31
View session
There was a problem hiding this comment.
Pull request overview
Syncs upstream changes from ppy/master, introducing a ruleset-level score multiplier calculator API (with per-ruleset implementations + tests/benchmarks), editor quality-of-life additions (slider velocity toolbox + new global hotkeys), and a small ranked-play matchmaking UI adjustment (recent match history capping).
Changes:
- Add
ScoreMultiplierCalculatorAPI, wire it intoRuleset, and implement per-ruleset calculators (osu/taiko/catch/mania) with unit tests and a benchmark. - Add an osu! editor “slider velocity” toolbox and use it during slider placement; adjust related editor toolbox naming.
- Add global editor actions + default keybindings for “submit beatmap” and “edit externally”, and cap matchmaking recent match panels.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| osu.Game/Tests/Rulesets/RulesetScoreMultiplierTest.cs | Shared test base for per-ruleset score multiplier validation. |
| osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs | Caps recent match history panels; adds (currently unused) state for ordering. |
| osu.Game/Screens/Edit/Editor.cs | Hooks new global editor actions and shows hotkeys in menu items. |
| osu.Game/Rulesets/Scoring/ScoreMultiplierCalculator.cs | New calculator API for mod multiplier computation (incl. combinations). |
| osu.Game/Rulesets/Ruleset.cs | Adds CreateScoreMultiplierCalculator() virtual factory. |
| osu.Game/Input/Bindings/GlobalActionContainer.cs | Adds new editor global actions + default keybindings. |
| osu.Game.Tests/Rulesets/Scoring/ScoreMultiplierCalculatorTest.cs | Unit tests for calculator behavior (flat/setting/context/combination). |
| osu.Game.Rulesets.Taiko/TaikoRuleset.cs | Wires taiko ruleset to taiko multiplier calculator. |
| osu.Game.Rulesets.Taiko/Scoring/TaikoScoreMultiplierCalculator.cs | Taiko-specific multipliers registration. |
| osu.Game.Rulesets.Taiko.Tests/TaikoScoreMultiplierTest.cs | Taiko multiplier regression tests. |
| osu.Game.Rulesets.Osu/Scoring/OsuScoreMultiplierCalculator.cs | Osu-specific multipliers registration. |
| osu.Game.Rulesets.Osu/OsuRuleset.cs | Wires osu ruleset to osu multiplier calculator. |
| osu.Game.Rulesets.Osu/Edit/OsuSliderVelocityToolboxGroup.cs | New editor toolbox group for slider velocity selection. |
| osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs | Adds the new toolbox group to the osu editor right toolbox. |
| osu.Game.Rulesets.Osu/Edit/FreehandSliderToolboxGroup.cs | Renames toolbox group identifier. |
| osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs | Uses toolbox-provided slider velocity on placement start. |
| osu.Game.Rulesets.Osu.Tests/OsuScoreMultiplierTest.cs | Osu multiplier regression tests. |
| osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderVelocityAdjust.cs | Adds editor UI test coverage for the new velocity toolbox. |
| osu.Game.Rulesets.Mania/Scoring/ManiaScoreMultiplierCalculator.cs | Mania-specific multipliers registration. |
| osu.Game.Rulesets.Mania/ManiaRuleset.cs | Wires mania ruleset to mania multiplier calculator. |
| osu.Game.Rulesets.Mania.Tests/ManiaScoreMultiplierTest.cs | Mania multiplier regression tests. |
| osu.Game.Rulesets.Catch/Scoring/CatchScoreMultiplierCalculator.cs | Catch-specific multipliers registration. |
| osu.Game.Rulesets.Catch/CatchRuleset.cs | Wires catch ruleset to catch multiplier calculator. |
| osu.Game.Rulesets.Catch.Tests/CatchScoreMultiplierTest.cs | Catch multiplier regression tests. |
| osu.Game.Benchmarks/BenchmarkScoreMultiplierCalculator.cs | Benchmarks calculator vs. multiplying Mod.ScoreMultiplier. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+18
| using osu.Game.Rulesets.Objects; | ||
| using osu.Game.Rulesets.Osu.Objects; | ||
| using osu.Game.Screens.Edit; | ||
| using osuTK; | ||
|
|
Comment on lines
+156
to
+160
| if (editorClock.CurrentTime != lastClockPosition) | ||
| { | ||
| sliderVelocitySourceObject.Invalidate(); | ||
| lastClockPosition = editorClock.CurrentTime; | ||
| } |
Comment on lines
+443
to
+444
| private int historyInsertOrder; | ||
|
|
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.
Syncs 9 commits from
ppy/masterinto our fork viagit merge -Xours(our changes take precedence on any conflict). Merge was clean — no conflicts.What came in
ScoreMultiplierCalculatorbase class inosu.Game/Rulesets/Scoring/, concrete implementations for all four rulesets, benchmark, and testsOsuSliderVelocityToolboxGroupwired intoOsuHitObjectComposerGlobalActionContainer)