Open
Conversation
Closes #36490. While I'm out here already taking heat for deleting mod combinations let me do more of that. The main problem with the mod combination here, again, is that the application of the mods does not commute. - The current behaviour is that TP is applied first, then DA. This is, again, "enforced" by the mod select overlay implicitly enforcing order of the mod instances in the global mod bindable to match the display order of mods inside it. Even this doesn't "work" correctly as is, because as the bug reporter points out, if they throw on DA with no changes expecting the map's default AR to be applied, it still gets halved. This is because DA works in a way wherein if you don't touch the AR slider, DA does not touch AR. Which means that the DA slider should *really* be at *half* of the map's base AR by default in this case because TP is active. How do you program this? - The *alternative* behaviour would be that DA is applied first, then TP. This in turn would mean that the effective range of AR adjustment offered by DA when TP is active would be halved to [0, 5] (or [-5, 5.5] with extended ranges). How do you program this? The above is just client-side concerns, while leaving out the other giant concern, which is "how do you get every single place in the game that may want to apply mods to a beatmap to apply them *in the same order*?". Then extend that to server-side components, then extend that to every external library that may want to re-implement SR/PP calculations, etc. etc. One additional remark: What the bug reporter *did not* say however, but I am saying, is that there's an elephant in the room, and that is the Easy mod, which *also* changes AR, but also *happens* to apply commutatively with Target Practice simply because both mods are implemented to halve the AR, which means that the order of application doesn't matter. If I were *really* bent on being a bad guy and just deleting mod combinations indiscriminately, I'd delete that one as well. But I'm not doing that.
Fix multiplayer team display becoming inconsistent
…36512) Probably closes #36492. This is dumb but it's in large part my stupidity. To begin with, the immediate direct offending call that causes the observed symptoms is https://github.com/ppy/osu/blob/a401c7d5e9d6d8b05b2ec293145ad308dfe9d6d0/osu.Game/Screens/Edit/Components/FormSampleSet.cs#L296 The reason why this "invalidation" affects sample volume is that in the framework implementation, the call [removes the relevant sample factory from the sample store which is an audio component](https://github.com/ppy/osu-framework/blob/5b716dcbef6f99e03188a7a7706361fa8445c754/osu.Framework/Audio/Sample/SampleStore.cs#L65-L72). In the process it also [unbinds audio adjustments](https://github.com/ppy/osu-framework/blob/5b716dcbef6f99e03188a7a7706361fa8445c754/osu.Framework/Audio/AudioCollectionManager.cs#L37-L38), which *would* have the effect of resetting the sample volume to 100%, effectively (and I've pretty much confirmed that that's what happens). Now for why this call sometimes does the right thing and sometimes doesn't: Sometimes the call is made in response to an *actual* change to the beatmap skin, which is correct and expected, if very indirect, but sometimes it is made *over-eagerly* when there is no reason to recycle anything yet. One such circumstance is entering the setup screen, which will still "invalidate" (read: remove) the samples, but the compose tab hasn't seen any change to the beatmap skin, so when it is returned to, it has no reason to retrieve the sample again, and as such it will try to play samples which are for better or worse in a completely undefined state because they're not supposed to be *in use* anymore. Therefore, the right thing here would seem to be to take the responsibility of invalidation from a random component, and move it to a place that's *actually* correlated to every other component needing to recycle samples, e.g. `EditorBeatmapSkin` responding to changes in the beatmap resources via raising `BeatmapSkinChanged`. Unfortunately, because of the structure of everything, this recycle needs to go from targeted to individual samples, to nuking the entire store. The reason for this is that `RealmBackedResourceStore` does not provide information as to *what* resources changed, it just says that *the set of them* did. For the recycle to be smarter, `EditorBeatmapSkin` would need to know not only which samples were added or replaced, but also which ones were *removed*, so that users don't hear phantom samples that no longer exist in the editor later. That would however be a lot of hassle for nothing, so I just recycle everything here and hope it won't matter. As to why I could only reproduce this on this one beatmap - I'm not super sure. The failure does not seem to be specific to beatmaps, but it may be exacerbated by certain patterns of accessing samples which means that beatmaps with high BPM like the one I managed to reproduce this on may just be more susceptible to this. As a final note, I do realise that this is not fundamentally improving the surrounding systems and it's still a pretty rickety thing to do. It's still on the consumers to know and respond to the sample store recycle and this is likely to fail if a consumer ever doesn't. That said, I have no brighter ideas at this point in time that won't involve me spending a week refactoring audio.
Regressed in d6bf4fd. One very visible instance of this regression is the login form. https://github.com/user-attachments/assets/5ba10ac5-4cb1-49af-b55c-89cf58ca0b44 The `CommentEditor` usage was discovered with one of my favourite tricks which is doing ```diff diff --git a/osu.Game/Graphics/UserInterface/OsuTextBox.cs b/osu.Game/Graphics/UserInterface/OsuTextBox.cs index fefe776..c17cca726b 100644 --- a/osu.Game/Graphics/UserInterface/OsuTextBox.cs +++ b/osu.Game/Graphics/UserInterface/OsuTextBox.cs @@ -42,6 +42,12 @@ public partial class OsuTextBox : BasicTextBox Margin = new MarginPadding { Left = 2 }, }; + public new bool Masking + { + get => base.Masking; + set => base.Masking = value; + } + protected bool DrawBorder { get; init; } = true; private OsuCaret? caret; ``` and then looking for usages of the setter. That's all due diligence you're getting here, I'm not auditing every single text box in the game. And yes, the `CommentEditor` usage is OMEGA dodgy but the change applied here is the only one that preserves its visual appearance. I'm not putting in time to fix it.
- depends on ppy/osu-framework#6700 - closes #36340 - supersedes and closes #36352 <img width="676" height="451" alt="image" src="https://github.com/user-attachments/assets/4f11c761-175b-495a-8b24-16fb6c481a15" /> --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
After testing out the icon on device for a bit, we decided the osu! logo was a little too large. This PR replaces the app icons with a new version where the sizing of the logo itself is aligned to Apple's recommended boundary. <img width="198" height="169" alt="Screenshot 2026-01-30 at 2 33 24 PM" src="https://github.com/user-attachments/assets/c66fb016-9c19-4ff5-814a-6f302ec459ca" />
- Closes #35743 Matches playlists. Multiplayer seems to not have it, but I think that's expected (i.e. there's no past scores, so having the initial position be "-" and no initial green color seems right). | Before | After | | --- | --- | |  |  | |  |  |
Fix unobserved timeouts still showing to user
Add skin cycling with shortcuts for next and previous skin
- Exiting while queueing will now background the search. - The "queue in background" button changed to "stop queueing". Exiting while in the "pending accept"/"waiting for players" states will exit from the queue. There is also a small period during the in-room state (where the matchmaking screen hasn't been pushed yet but "good luck" is displaying) where the user cannot exit from the screen. I've removed the exit confirmation dialogs to streamline the process and align with this. https://github.com/user-attachments/assets/8c172502-0624-42cd-ae0c-bb710068267c
Fixes #36562 Currently there exists a bug where the mod preset hotkeys will wrap outside of the intended bounds. Fix is making sure the preset index is < 10 Before: <img width="486" height="358" alt="image" src="https://github.com/user-attachments/assets/77f1ca9e-4fd0-4b29-b9f5-f53e652db42d" /> After: <img width="1007" height="1295" alt="image" src="https://github.com/user-attachments/assets/ca81cc39-2f86-462c-a26b-002aceed77f4" />
Fix revert to default button not resizing correctly after changing languages
- Adds sorting and display styles. - Saves sort/display modes to the config. - Improves performance, especially on the 2nd+ time opening the overlay. https://github.com/user-attachments/assets/e32b50d0-58a1-4eef-b18c-988fb497e545 --- Coming off some recent feedback in #33426 (comment), I decided to take a bit of a detour and get a little bit more functionality in. Sorting by rank, although it should technically work, doesn't work right now. This is because the osu!web API doesn't return user rank on `/user/` lookups - it's only returned for the friends request. I'm leaving this open as a discussion topic. - We can make osu!web return the rank and osu! will require no further changes to work correctly, or - We can try to implement additional paths through `osu-server-spectator` which would blow this PR out of proportion and is best left for a task of its own. For simplicity, I've re-implemented this display mostly as its own component for now, lifting code from `FriendDisplay` which was recently overhauled. These implementations should eventually be combined somehow but that's dependent on: 1. Figuring out the styling - friends can display offline users for which it makes no sense to display the "spectate" button. 2. Figuring out how to handle the different users/presence pathways. It's mostly a code complexity issue. --------- Co-authored-by: Dean Herbert <pe@ppy.sh> Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
…entering song select (#36601)
…ly (#36564) my attempt at #36452: changes difficulty range slider in song select V2 to use the new star difficulty text gradient colors from #36292. closes #36405 https://github.com/user-attachments/assets/134ace54-a8f8-4024-a32e-f1604c868232 to match star rating more closely for sr <= 7.5 (where i don't think the colors are so intense that they break harmony with the rest of the design), i removed the Lighten by 0.4f on color update also improved the transition from 7.9 to 8.0 which is quite abrupt in the current version #33425 (comment) is relevant here
…missions to do what it claims to do (#36602)
Grouped notifications for more than 1 person were added in #36180 but it looks like they forgot to add the Transient and IsImportant flags, which means the grouped notifications would still stay in the notification list/flash the taskbar. Before: https://github.com/user-attachments/assets/8a34bbc0-2b5c-4086-b2ee-1daa6d1e6e10 After: https://github.com/user-attachments/assets/03c25ba6-7c8e-464c-bbb1-688ab9da6bb6
…6616) - Closes #35389 Same as: https://github.com/ppy/osu/blob/2efe0c95e63817f312f5fb12cc60dd56bee0023b/osu.Game/Screens/Edit/Editor.cs#L1173-L1180 There's also seeking hit objects and sample points, but the seeks are relatively close to each other and probably useless when playing(?). If we want to make those cases not stuck at the same point in time, I believe the leniency should be lower than 1000 ms. With the above, that is why I just copy-pasted the code, as we may want to have different leniencies. Edit: forgot the automated label thing, will not label next time
## [Specify `Accept` header in registration request](28edb78) The lack of it meant that in specific scenarios web would respond with a chunk of HTML instead of JSON. ## [Allow showing registration error message even if no redirect is given](6ad4994) There are scenarios where this can happen, and if it did, previously the strict requirement to have both would cause the specific message to be discarded and replaced with the generic "something happened" one.
…d beatmap is online (#36632) - Closes #36584 The last two commits could be either fixes to the issue above, but in a code quality perspective, the scheduler in `setLink()` seems unnecessary as the other set methods don't have it (other than making it run last) and the other commit is self explanatory.
Let's try and find a sweet spot somewhere between lazer and stable that we can all be happy with 🙃.
Reported [on discord](https://discord.com/channels/188630481301012481/1097318920991559880/1462402881658294355). Not sure if this has been turned into an issue anywhere. Fixes awkward looking padding at the bottom of `FilterControl` by having `ScopedBeatmapSetDisplay` apply the top padding on its own instead of relying on the spacing of its parent fill flow. This way, the padding is animated away when the scoped display disappears, instead of remaining because the component is still technically there. | Before | After | |--------|--------| | <img width="1123" height="247" alt="image" src="https://github.com/user-attachments/assets/edadd97e-cada-4378-98b7-cc85bfa01fb5" /> | <img width="1120" height="246" alt="image" src="https://github.com/user-attachments/assets/d8daecd0-1dfc-4594-95a4-94327677da89" /> | [Screencast_20260325_114439.webm](https://github.com/user-attachments/assets/99bb7083-b2da-40a3-b5a0-d94476e7c1ac) I don't really like how `ScopedBeatmapSetDisplay` is now managing its own spacing in the parent container, but it's tightly tied to the filter control anyway so it's probably fine.
- Simplifies track handling by not attaching to the global beatmap, not looping, and moving into `GameplayWarmupScreen` where the beatmap is set. - The main idea here is that the transition period until gameplay is so short (~10 seconds) that we don't need to account for the track ever looping in the first place. - Fixes the track not playing from its preview point (feedback item mentioned in some meeting a while back). This is definitely going to conflict with @nekodex 's work, sorry about that. It's a bit of a much-of-a-muchness change (imo) if the conclusion is to wait for ongoing work first.
…eplace background is disabled (#37112) Man this "storyboard replaces background" baloney has taken hours of bugfixing alone. So many forehead indentations from stepping onto this stupid rake. This still fails in one more case: when you download a no-video variant of a beatmap that has video, but then edit it, all of the flags on storyboard will claim that the beatmap has a storyboard that replaces a background, but the video asset is missing, so the background will still be black. There's currently no way to check for this and the simplest way to address this as far as I can see would be reverting #37038 and going with the non-refactor route to fix #36875 instead. The alternative is adding all sorts of weird jingles and checks in the storyboard machinery that can be used to be able to tell that a video was supposed to be present in the storyboard but is missing. Also when entering editor on a map that has background video and storyboard enabled the background will be black until you hit play. Something to do with `Video` idiosyncrasies for sure. Closes #37104 maybe? Kind of? Partially? I don't know. This is all very low effort because I'm not confident about digging this ditch any deeper, but just PRing a direct revert would feel pretty offensive I guess?
It's just something more than nothing. <img width="1079" height="554" alt="osu Game Tests 2026-03-27 at 08 26 34" src="https://github.com/user-attachments/assets/de1485f7-b29c-4bc5-aec4-dbc736a87961" />
…o already installed location (#37137) - Closes #37039 https://github.com/user-attachments/assets/eb788984-783c-42ac-a7db-e6daa64d609d Can't gracefully exit because main menu has another exit confirmation dialog. May need to revisit #26558. Unless force exiting is intentional?
…t) (#37129) This was mentioned in vivi's feedback. Basically now the card backside is always loaded and there, rather than faffing with switching the content around. > Cards have a stale grey background when they are spawning in before they get changed into the cards they’re supposed to be. This can be changed to the backside of the card or maybe a bright white card. Makes it look less placeholdery. Keeping it simple for now. Can probably hide it when not in use in the future.
Missed this in the earlier PR. As on [discord.](https://discord.com/channels/188630481301012481/1097318920991559880/1488164048976609420)
In discussions, we've come to the conclusion to attempt to use a chat-bubble system to minimise the effective area of the chat. In particular, the results screen doesn't give us enough space to display the full chat box without overlapping the main screen content. This PR both adds the chat to the results screen, and reworks it to use such a bubble system (not sure what to call it, IM style?). https://github.com/user-attachments/assets/a8a88c51-8a9d-4a03-92b6-621112a15a41 - New messages are previewed for 3 seconds. - When focusing and unfocusing the textbox, the history moves into expanded state (show the most recent 10 messages) or collapsed state (fade messages out ASAP). This is a bit of an initial implementation to get a feel of how it behaves, and there's more that can be done such as adding colours, improving the transforms, perhaps adding it to the intro screen (post-animation) but the structure's a bit weird atm. --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
- Closes #37055 In the editor, keybinds and the tooltip for the hitsounding section are hardcoded. Since the keybind contains "Alt", this is inconsistent on MacOS where "Opt" is used instead. Before: <img width="191" height="244" alt="image" src="https://github.com/user-attachments/assets/749f9dd1-f037-4061-848e-44161913190d" /> After (MacOS only): <img width="193" height="244" alt="image" src="https://github.com/user-attachments/assets/c807a23d-225e-41ab-993a-df9230be58bc" /> --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
The fix is just disabling the animation. It works I guess. --- - Closes #37042 Currently in Mania, you can change the scroll speed for a brief period during the beginning of a song. However this scroll speed change occurs over a short period of time, which causes a bunch of extra hit object updates, causing major fps and latency drops. This fix simply replaces the dampening with an immediate scroll speed update. Since the scroll speed can only be updated for a short time at the beginning of the song, providing immediate visual feedback on the scroll speed makes sense to me. However another potential solution would be to filter the TimeRange Value updates to keep the gradual scroll speed visual change, while greatly reducing the number of updates to the hit objects currently on screen. If there is any feedback I would greatly appreciate it as this is my first issue here. I had ran both inspectCode.ps1 and the code formatter before creating the merge request. Thank you. Before fix: https://github.com/user-attachments/assets/55e30894-7341-414a-af2e-2ec051c3a252 After fix: https://github.com/user-attachments/assets/c085d33f-c0ae-45dd-8131-e79a5682b9ca --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
Adds the ability to drag and reorder cards. Card order is preserved between rounds and is synchronized between both players (each player can see the other player drag around and reorder their cards). To make this possible I had to rewrite the card layout algorithm to be stateless (e0a46fe), there wasn't really a way around that since I needed a way to calculate a layout position based on a card's index. Should hopefully be a lot easier to read now though. Some noteworthy stuff: - I didn't really know what the best place to store the card order is, so I put it on `RankedPlayCardWithPlaylistItem` since that one will stay the same instance per round. - To prevent the opponent's cards from dragged into the middle of the playfield, only the x axis of the drag gets synchronized for the `OpponentHandOfCards` with a fixed y value. - I adjusted the replay recorder/player parameters a little. With the drag events happening every frame the replay recorder record new frames every 25ms and end up dropping half the replay frames per flush interval, so I increased the sample interval to 50ms so the buffer size matches the sample rate exactly (50ms -> 20 samples per flush every 1000ms). I also increased the buffer size in the replay player a bit so slight fluctuations in latency won't make it start to drop frames. https://github.com/user-attachments/assets/b810cb85-db02-4edf-a63e-bfc96cf59665 https://github.com/user-attachments/assets/4d2f884d-fcce-4948-9659-fbb314634cb8 --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
Intended to add toggle but forgot. This also fixes #37012 via a convoluted refactor of a lot of stuff. The basic overview is: - Moved all replay overlay concerns out of `HUDOverlay`. We can display this above everything confidently (i think). - Split out `ReplayOverlay` and `ReplaySettingsOverlay` so the base class can handle the visibility, hotkeys and everything that should be shared with *all* replay overlay components going forward. `Ctrl+H` is supposed to hide any of these kinds of details, and I'm sure we'll add more in the future. - Reorganised some things in `Player` so the new structure would work. Mainly the overlays which add a black layer during fade out.
- Closes #31684 Uses the global action variant of up and down for dropdown menus. It is already used in multiplayer/playlist room/beatmap navigation and gameplay menu overlays. Comment wording is derived from: https://github.com/ppy/osu/blob/fc817627e56b7db1c46e2f00fc9a3bd14af51211/osu.Game/Graphics/UserInterface/FocusedTextBox.cs#L73-L74
Noticed this not updating when working on tests.
Using a bespoke music loop, fades in during the intro: https://github.com/user-attachments/assets/8db7e30c-2a7c-4fd0-8385-fa77a6e0a506 Ducks when hovering cards for previews, with debouncing to smooth it: https://github.com/user-attachments/assets/b3557c08-c675-4591-963c-95866d45107f --- - [x] depends on ppy/osu-resources#414 --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
- Fixes CI failures due to completely wrong test (sorry) - Fixes #37167 (oops)
Mod that colours HitObjects based on the musical division they are on, now in osu!catch https://github.com/user-attachments/assets/2dda493d-dc8b-4ea4-ba47-7d04e2062b42 For now *bananas are not coloured by the mod and keep their yellow colour*, since I think its better for the sake of readabilty (also it just looks kinda ugly?). Do leave your thoughts on that. Droplets are always coloured to `LightGreen`, setting their colour to closest timeline ticks is wrong and looks incorrect since droplets aren't generated with them in mind. --------- Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com> Co-authored-by: Shavix <54279284+Shavixinio@users.noreply.github.com>
## [Adjust CI test reporting to upstream action changes](f736337) It's been semi-broken since I bumped it a few weeks ago. Paper trail for this is at dorny/test-reporter#750, but to TL;DR it: `dorny/test-reporter@>=v2.0.0` migrated from [creating new check runs via the GitHub API](https://docs.github.com/en/rest/checks/runs) to [job summaries](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#adding-a-job-summary) by default. The main difference is that compared to check runs, job summaries do not *appear to* require extra `GITHUB_TOKEN` permissions, but in exchange are limited to the job that called it. This broke visibility of the test reports because due to `GITHUB_TOKEN` permissions foibles the test reporter was running in a separate workflow. I see migrating as a plus here, since: - The visibility of results is comparable-to-better (example available for preview at https://github.com/bdach/osu/actions/runs/23892493152) - No longer required to have a completely separate workflow for test result reporting - No longer required to give `checks: write` permissions to the action (I'd hope, we'll see, untested on a public repository with PRs involved) One downside is there'll be no in-code annotations for failing tests anymore but that's whatever IMO. Half the time they weren't even very helpful, test results pretty much require maintainer interpretation anyway. This needs to be applied to a few other repos but I'm starting here because this is the one where the traffic is highest and therefore unbreaking the report is of most value (and also the one where I'll see if it works with public PRs the fastest). Side note, I was hoping to remove the artifact upload/download games by just attaching the summary inside each individual test job in the matrix, but [it looks like crap](https://github.com/bdach/osu-framework/actions/runs/23888384309) because only the first three summaries are loaded by default, so if there are more, you have to click each remaining one to see its output. Wow. Awesome. Also updates the action to `v3.0.0` to resolve node deprecation warnings. ## [Update inspectcode version to resolve deprecation warnings](496cf68) More node deprecation warning fixes.
…me (#37152) - Last part of / closes ppy/osu-server-spectator#406. - Remaining work on slots will be tracked in ppy/osu-server-spectator#405. This PR is a corollary of ppy/osu-server-spectator#453 and all of the dispensations referee users in a multiplayer have received therein. The goal here is to allow access to all relevant room management functions even if the referee in question isn't host, as well as to disallow access to all non-relevant functions to do with the actual match gameplay. I'm not going to lie, this logic *is* ugly. I would argue that it already *was* ugly on `master` and my goal was to operate with as light a touch as possible myself. But you could see this as copping out and that I should try to refactor some of this. I will try - but only after someone else's seen the initial approach and deemed it unsuitable. The logic in `MatchStartControl` is awful - there are so many moving pieces of state that dictate what can happen when with all the buttons, and yes, I am making it worse here. This time there is some test coverage. Not everything is covered, but the coverage should be on par in all components and pieces of relevant logic I touched that already had tests covering them. On that note, please forgive the diffstat size, but the tests *are* most of that size. --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
) Exposed by CI failures ([example](https://github.com/ppy/osu/actions/runs/23888446400#user-content-r0s0)). The race occurs when a consumer calls `GetBindableDifficulty()` for the first time and then a cache invalidation is triggered. The sequence of events triggering the failure is as follows: 1. Consumer calls `GetBindableDifficulty()` to get a difficulty bindable for a given beatmap tracking the game-global ruleset / mods. This triggers difficulty calculation A. 2. In the meantime, another process requests a cache invalidation for the same beatmap as the one supplied by the consumer in step (1). This incurs a cache purge and triggers difficulty calculation B, but never cancels difficulty calculation A. 3. Difficulty calculation B concludes and writes the correct, latest difficulty value to the bindable. 4. Difficulty calculation A concludes and writes an incorrect, stale difficulty value to the bindable. See below for patch that reproduces this behaviour on my machine 100% reliably: ```diff diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index https://github.com/ppy/osu/commit/d6b40639161e26af223f03761b3826b0cd7f4a87..c9604e0e58 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -252,17 +252,17 @@ private void updateBindable(BindableStarDifficulty bindable, IRulesetInfo? rules GetDifficultyAsync(bindable.BeatmapInfo, rulesetInfo, mods, cancellationToken, computationDelay) .ContinueWith(task => { + StarDifficulty? starDifficulty = task.GetResultSafely(); + // We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events. - Schedule(() => + Scheduler.AddDelayed(() => { if (cancellationToken.IsCancellationRequested) return; - StarDifficulty? starDifficulty = task.GetResultSafely(); - if (starDifficulty != null) bindable.Value = starDifficulty.Value; - }); + }, starDifficulty?.Stars > 0 ? 400 : 0); }, cancellationToken); } ``` The goal of the patch is to reorder the write to the bindable in order to trigger the scenario described above. Due to the invasiveness of the patch it is not suitable to add as a test, and chances are that the schedule delay may need to be tweaked if anyone else wants to check that patch. Anyway, the solution here is to use the same pattern of creating a linked cancellation token even on the first retrieval of a bindable difficulty, and registering it in the list of cancellation tokens that already existed to service the ruleset- / mod-tracking flow. Some extra rearranging in 9184299 is needed to ensure the linked tokens created to do this don't stay behind after they are no longer needed for anything.
Example: https://github.com/ppy/osu/actions/runs/23900675414/job/69696255970?pr=37178#step:5:38 Regressed in #37172, cc @LiquidPL Would fail in multiple tests. I'm not going to spend time figuring out exactly why, I'm just going to guess that not all tests bother to set up the relevant playlist items for the cards or whatever. Some of the failing tests are flaky but not because the `item` here isn't sometimes null in those cases. It's always null, but the callbacks are probably scheduled or whatever and therefore have a chance to never run. Also some of the failures appear to cascade / spill from other tests as well.
A few quick ones from #37190. ## [Rewrite `TestSceneDeleteLocalScore` to have less context menu containers (and hopefully no longer flake)](ea0bc5b) As seen in https://github.com/ppy/osu/actions/runs/23890777748#user-content-r3s3. This is a speculative fix but I'm feeling somewhat confident about this one. `BeatmapLeaderboardWedge` has TWO separate `ContextMenuContainer`s itself, and the test mentioned here was bringing a third. I have a feeling that the test flaking may have something to do with the fact that the test logic would attempt to click a menu item on specifically ONE of the three context menus. My bet is that when it fails, it's because it's trying the wrong one, but I don't have reproduction. ## [Wait for text to appear in flaking `TestSceneDrawableRoomPlaylist.TestSelectableMouseHandling` test](9d62eea) As seen in https://github.com/ppy/osu/actions/runs/23888446400#user-content-r1s1. Speculative. Banking on the fact that it takes time to load the sprite texts. ## [Remove all tests from `TestSceneWikiMarkdownContainer` dependent on existence of wiki on dev](8f319f9) Deletes a flaky as seen in https://github.com/ppy/osu/actions/runs/23878899702#user-content-r2s2. The year is 2026 and LLM scrapers hammer [the entire](https://sourcehut.org/blog/2025-04-15-you-cannot-have-our-users-data/) [internet](https://blog.metabrainz.org/2025/12/11/we-cant-have-nice-things-because-of-ai-scrapers/) all over to scrape whatever ounce of Human Content there is left to feed the Moloch so that it can regurgitate it back in the form of The Most Average Speech You've Ever Read. We are not immune to this, and as such the LLM homunculi have hit the dev.ppy.sh wiki instance enough times for it to just completely [be banished to the blagole](https://www.youtube.com/watch?v=AfA_2Ku1aJY). Which means I get to freely delete flaky tests that should never have been running as part of CI because they're completely useless now and it's not like we're ever turning them back on again. ## [Use equality check in `TestSceneBeatmapCarouselScrolling.TestScrollPositionMaintainedOnRemove_SecondSelected` that's less sensitive to floating point](6f2a1de) As seen in https://github.com/ppy/osu/actions/runs/23826420794#user-content-r0s1.
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
20 participants
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.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )