Addresses https://github.com/ppy/osu/discussions/36004.
Not adding localisation because the previous implementation was
`.ToString()`ing anyway.
Would have made the abuse e-mail a link but `mailto:` doesn't work with
`MessageFormatter` and I don't want to go into that right now.
The message *almost* matches stable. The "almost" is because it doesn't
mention the `/ignore` chat command. I was just going to implement the
command, but I went to check what it does, and backed away slowly
because it has like weird scoping to chat, highlights, and PMs, so
`nope.avi`.
Closes https://github.com/ppy/osu/issues/36003.
The duplicated `RulesetSkinProvidingContainer` is unfortunate but it's
either this or I start doing proxy shenanigans.
"Closes" https://github.com/ppy/osu/issues/35920.
The button can't easily work anyway since it's not guaranteed that the
spectating user has all of the frames of the replay (think entering
spectate midway through a play).
This matches the results screen in spectator too.
* Add failing test for copy->paste not being idempotent
* Ensure all elements on default skins use fixed anchors
`UsesFixedAnchor` defaults to false, i.e. closest anchors. Combined with
manual anchor / origin specs on some drawables, this would get default
skins into impossible states wherein a drawable would use "closest
anchor" but also explicitly specify anchor / origin that aren't closest,
which horribly fails on attempting to copy and paste.
Frankly shocked this has gone unnoticed for this long, and I regret not
vetoing this "feature" more every time I see its tentacles spread to
produce breakage of levels yet unseen.
Does this commit contain major levels of suck? For sure. Do I have any
better ideas that wouldn't consist of a multi-day rewrite or deletion of
this "feature"? No.
* Fix skin editor always applying closest anchor / origin on paste regardless of whether the component uses fixed anchor
Self-explanatory. Should close https://github.com/ppy/osu/issues/29111
along with previous commit.
More or less covers the first page of client sentry issues sorted by
volume, all of which is pretty much useless for anything because it's
client-specific-failure noise.
More or less covers the first page of client sentry issues sorted by
volume, all of which is pretty much useless for anything because it's
client-specific-failure noise.
Unsure about this one, but I find the preceding commit to be very
lacking in explaining to the user why the editor don't work. Shining
some things red may help aid understanding.
Idk why, but in my first commit here I just commented out the lines
restricting DB schema suffixes to debug builds only, and before all that
mess there was a "TODO: fix".
I'm only doing this for sake of tools like BeatmapExporter and to not
clog up disk space when newer schema versions arrive.
This should work well with existing installations (hopefully)
Closes https://github.com/ppy/osu/issues/35807.
The reason this closes the aforementioned issue is as follows:
Taking https://osu.ppy.sh/beatmapsets/1236180#osu/4650477 as the
example, we have:
```
minBeatLength = 342.857142857143
maxBeatLength = 419.58041958042003
mostCommonBeatLength = 342.85700000000003
```
Note that `mostCommonBeatLength < minBeatLength` here.
Taking the inverse of that to compute BPM, we get
```
minBpm = 174.99999999999991
maxBpm = 142.99999999999986
mostCommonBpm = 175.00007291669704
```
which without DT present doesn't do anything bad, but when DT is
engaged (and thus BPM is multiplied by 1.5), midpoint rounding causes
the min BPM to become 262, and the most common BPM to become 263.
* Fix mania editor timestamp generation being culture-dependent
Mostly closes https://github.com/ppy/osu/issues/35809.
* Add failing test for notes with fractions
* Round note time when copying out timestamp & apply half-millisecond tolerance when parsing
Closes the rest of https://github.com/ppy/osu/issues/35809.
One issue here was that while the timestamp generation would allow
fractional object timestamps to be output, the parsing (via
`selection_regex`) would *reject* fractional timestamps, therefore
making lazer incompatible even with itself.
The other is that rounding is probably fine to do anyway for
interoperability with stable. I'd hope nobody actually *needs*
sub-millisecond precision but I'm ready to be proven wrong by some
aspire jokester.
* Specify invariant culture when writing out combo indices to editor timestamp in other rulesets
Pretty sure this is just a much-of-muchness because it's integers but
might as well if I'm spending time here already.
Addresses https://github.com/ppy/osu/discussions/35811 I guess.
Will only work for local leaderboards for now but maybe good enough for
what is essentially a 5 minute job?
Can be made to work with online leaderboards too I guess if need be.
This is dodgy as hell but `ShortName` is completely derived from
`OnlineID` anyway so there should be no valid reason to ever attempt to
serialise it anyway.
Intended to match the rest of the UI which is less rounded these days.
See inline comment for reason for not matching `FormControl` corner
radius just yet.
Closes https://github.com/ppy/osu/issues/35721.
I worry that straight up removing the nuke and not adding any channel
leave calls in exchange is going to leave tourney client users
with the *inverse* problem of being joined into a gorillion channels
from multiplayer matches they broadcasted, so this attempts to strike a
reasonable balance.
* Load all beatmaps in bulk for SubScreenBeatmapSelect
* Fix tests no longer working due to drawable changes
* Remove test that no longer makes sense
* Split matchmaking panel into subclasses for each panel type
* Adjust tests to match new structure
* Add `ConfigureAwait`
* Display loading spinner while beatmaps are being fetched
* Fix test failure
* Load playlist items directly in `LoadComplete`
* Convert `MatchmakingSelectPanel` card content classes into nested classes
* Wait for panels to be loaded before operating on them
* Add ConfigureAwait()
---------
Co-authored-by: Dan Balasescu <smoogipoo@smgi.me>
* Add failing test coverage for layered hit samples not playing in mania when beatmap is converted
Adding the `osu.Game.Rulesets.Osu` reference to the mania test project
is required so that `HitObjectSampleTest` base logic doesn't die on
f0aeeeea96/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs (L88-L91)
* Fix layered hit sounds not playing on converted beatmaps in mania
Compare
f9e58b4864/osu!/GameplayElements/HitObjects/HitObject.cs#L476-L477.
In case of converted beatmaps, the last condition there
(`BeatmapManager.Current.PlayMode != PlayModes.OsuMania`) fails,
and thus layered hitsounds are allowed to play.
* Add failing test coverage for mania beatmap conversion assigning wrong samples to spinners
* Fix mania beatmap conversion assigning wrong samples to spinners
A spinner is never `IHasRepeats`. It was a dead condition, leading to
the hitobject generating fallback `NodeSamples`, which in particular
feature a silent tail which stable doesn't do.
Noticeably, stable also appears to force the head of the generated hold
note to have no addition sounds:
f9e58b4864/osu!/GameplayElements/HitObjects/Mania/SpinnerMania.cs#L86-L89
* Add failing test coverage for file hit sample not falling back to plain samples if file missing
* Allow `FileHitSampleInfo` to fall back to standard samples if the file is not found (or not allowed to be looked up)
I'm honestly not 100% as to how closely this matches stable because I
reached the point wherein I'd rather not look at stable code anymore, so
as long as this passes tests I'm fine to wait for someone else to report
new breakage.
* Use alternative workaround for lack of osu! ruleset assembly in mania test project
* Fix encode stability test failures
Probably closes https://github.com/ppy/osu/issues/35650.
Realm slow, episode 23894. I can't reproduce freezes as big as the video
in the issue is showing but 'realm slow' is 99% the culprit, because
affected user's database is not small.
Because I just wasted 30 minutes trying to debug why a skin provided by
a user in an issue thread was failing to deserialise, only to realise
halfway through that the deserialisation error I was seeing was *from
the fallback path and thus a complete red herring*.
Closes https://github.com/ppy/osu/issues/35651.
The reproduction steps provided in the issue are too complex even. In my
testing all you need to do is go into editor, replace the background via
external editing, and exit out to song select; you'll immediately see
loss of selection on the carousel, the set panel still using the old
background, and eventually a crash when you attempt to re-select any of
the difficulties of the edited set.
`HandleItemsChanged()` - an optimisation aiming to reduce the number
of redundant re-filters due to minor changes to realm models that aren't
visible to the user anyway - ignoring changes to `BeatmapInfo.ID` after
re-entering song select post-external edit meant that song select would
retain stale beatmap models that no longer existed in the realm
database, thus failing refetch attempts via `GetWorkingBeatmap()` or
8f6f859c15/osu.Game/Screens/SelectV2/FooterButtonOptions.cs (L56-L57)
RFC. Written to address
https://osu.ppy.sh/community/forums/topics/2150023.
Few other things we might want to happen here:
- pause the track when starting the drag
- figure out what to do when a drag is held while the track changes in
the background (which was impossible to happen before this)
but I want to see the reaction to this first.
Clicking the button opens the browser, on the "new topic" page inside
the help forum. Web can now correctly read the build number of the
client since https://github.com/ppy/osu-web/pull/12478 so I see
no reason not to.
Minimal effort implementation. Stemmed from discussion in
https://discord.com/channels/90072389919997952/299846395031060480/1437368033734561792.
Not really interested in putting more effort into this at this point, if
this is not considered acceptable then just close the PR and this can be
revisited more properly at a later date.
* Add failing test coverage for blocking users not removing their messages from public channels
* Fix messages from blocked users being visible in public channels
Closes https://github.com/ppy/osu/issues/35633.
It appears that the expectation from web here is that messages from
blocked users should be excised client-side. Compare:
12dd504255/resources/js/chat/conversation-view.tsx (L104)
This implementation won't *restore* the messages after a block and
unblock, but I kind of... don't care if I'm honest with you? Making that
happen will result in a bunch of complications for no reason, so I'm
fine waiting for anyone to complain about it.
RFC, lowest effort solution for https://github.com/ppy/osu/issues/34979.
The `SkinImporter` conditional *is* hella ugly, but anything less ugly
will require taking a hammer to structures. Maybe passing version via
the import flow, maybe even trying to make the `EnsureMutableSkin()`
flow somehow attempt to read the `skin.ini` that's in resources. No
idea.
Properties from `skin.ini` that were defaults or that lazer can't
(won't ever?) understand snipped.
`VerificationFailureResponse.RequiredSessionVerificationMethod` not
being nullable means that if it was missing in the verification
response, it would not be `null` but default to `TimedOneTimePassword`
instead, therefore showing TOTP-related error messages to users that
never enabled it rather than the user-facing message they were supposed
to.
Most easily tested on a local full-stack environment with
```diff
diff --git a/app/Libraries/SessionVerification/MailState.php b/app/Libraries/SessionVerification/MailState.php
index 305a2794ec0..3c2d15f335b 100644
--- a/app/Libraries/SessionVerification/MailState.php
+++ b/app/Libraries/SessionVerification/MailState.php
@@ -14,7 +14,7 @@ use Carbon\CarbonImmutable;
class MailState
{
- private const KEY_VALID_DURATION = 600;
+ private const KEY_VALID_DURATION = 10;
public readonly CarbonImmutable $expiresAt;
public readonly string $key;
```
applied so that you don't have to wait 10 minutes to trigger the
failure.
Issue was bisected to [this commit](6f1664f0a6)
This change in the commit outlined is what caused the issue:
```diff
BreakOverlay = new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks, ScoreProcessor)
{
Clock = DrawableRuleset.FrameStableClock,
ProcessCustomClock = false,
- Breaks = working.Beatmap.Breaks
+ BreakTracker = breakTracker,
},
```
`BreakTracker` always initializes breaks as `new Period(b.StartTime, b.EndTime - BreakOverlay.BREAK_FADE_DURATION);` leaving room at the end to account for the fade before resuming gameplay.
Because of this, changing the `BreakOverlay` to use a `BreakTracker` instead of the original beatmap breaks caused each break to be `BREAK_FADE_DURATION` shorter than it was originally - which in this case is 325ms - leading to the discrepancy between the background fadeout and the overlay fadeout.
Since the current behavior is 'correct', aligning the overlay with the rest of the beatmap such as background fadeout, I changed the timing to account for the shorter duration instead of revert the overlay initialization.
* Add new API property backing for tiered rank
* Slightly refactor `ProfileValueDisplay` for direct access to things that will need direct access
* Extract separate component for global rank display
* Add tiered colours for global rank
* Add Github link button to wiki overlay header
* Localize jump link string
* Mark ILinkHandler dependency as nullable
* Make the button actually look like it does on the website
* Use existing web string instead of inventing a new one
* Bind value change callback more reliably
---------
Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
This was primarily written to fix
https://github.com/ppy/osu/issues/35538, but also incidentally targets
some other scenarios, such as:
- When switching from artist filtering to title filtering, selection
sometimes would stay at the group under which the selection's artist
was filed, rather than moving to the group under which the selection's
title is filed (in other words, the group that *the selection is
currently under*).
- When simply assigning a beatmap to a collection such that it would
be moved out of the current group, the selection will now follow to
the new collection's group rather than staying at its previous
position.
Whether this is desired is highly likely to be extremely situational,
but I don't want to introduce complications unless it's absolutely
necessary.
This has a significant performance overhead because
`CheckModelEquality()` isn't free, but it doesn't seem horrible in
profiling.
Calling `HandleItemActivated()` rather than its intended 'parent method'
of `Activate()` meant that selection state was not correctly
invalidated:
819da1bc38/osu.Game/Graphics/Carousel/Carousel.cs (L157)
which in turn meant that carousel item Y positions would not be
recalculated correctly after the group was expanded, which meant that
the items would become
- visible,
- stuck to the bottom of the expanded group,
- one on top of another.
Which is not something that's going to perform well.
Certified OOP moment.
Fell out when attempting
https://github.com/ppy/osu-server-spectator/pull/346.
Functionally, if a true non-`HubException` is produced via an invocation
of a spectator server hub method, this doesn't really do much - the
error will still log as 'unobserved' due to the default handler, it will
still show up on sentry, etc. The only difference is that it'll get
handled via the continuation installed in `FireAndForget()` rather than
the `TaskScheduler.UnobservedTaskException` event.
The only real case where this is relevant is when the server throws
`HubException`s, which will now instead bubble up to a more
human-readable form. Which is relevant to the aforementioned PR because
that one makes any hub method potentially throw a `HubException` if the
client version is too old.
Obviously this does nothing for the existing old clients.
* Apply some renames & drawable names for visualiser
Optional but really helps me make heads of tails as to what anything is
here.
Like really, multiple variations of `footerContent` inside a
`ScreenFooter` class, with zero elaboration that it's really content to
do with *overlays*...
* Fix screen footer overlay content being pushed to right during fade-out
- Closes https://github.com/ppy/osu/issues/35203
- Supersedes / closes https://github.com/ppy/osu/pull/35468
Closes https://github.com/ppy/osu/issues/35003.
Bit dodgy to use `CurrentSelectionItem` for this. Ideally I would use
the global `Beatmap.IsDefault`, but I kind of don't want to violate the
rule that `BeatmapCarousel` shouldn't have direct access to the global
beatmap. And this seems to work, so... maybe fine to use until it
doesn't?
This is actually possible in current usages if you e.g. toggle "use
original metadata" on/off which will change the width of the underlying
sprite texts. Or by setting window size. Pick your poison.
Instead of truncating.
Addresses https://github.com/ppy/osu/discussions/35404.
The one "tiny" problem is that the "click to search" functionality of
these texts is maybe a bit worse now, because the clickable target is
now the full width of the wedge rather than autosized to the text.
Salvaging this is *maybe* possible, but *definitely* annoying, so I'd
rather not frontload it.
Is this lazy? Sure it is. Friends and blocks do the same thing, though,
and I'm not overthinking this any more than I already have.
Being smarter here would likely mean being more invasive with respect to
listening in on all outgoing API requests and silently updating
favourites on that basis. Which is "smart" but also complicated.
Something I've asked to be done for a long time. Relevant because I've
complained about this on every addition of a new piece of user-local
state: friends, blocks, and now favourite beatmaps.
It's just so messy managing all this inside `APIAccess` next to
everything else, IMO.
The "partial" leaderboard logic in `SoloGameplayLeaderboardProvider`
always assumed the online fetch would request 50 scores, which is no
longer the case after https://github.com/ppy/osu/pull/33100.
This is a set of model changes which is supposed to facilitate support
for custom sample sets to the beatmap editor that is on par with stable.
It is the minimal set of changes. Because of this, it can probably be
considered "ugly" or however else you want to put it - but before you
say that, I want to try and pre-empt that criticism by explaining where
the problems lie.
Problem #1: duality in sample models
---
There is currently a weird duality of what a `HitObject`'s samples will
be.
- If an object has just been placed in the editor, and not saved /
decoded yet, it will use `HitSampleInfo`.
- If an object has already been encoded to the beatmap at least once, it
will use `ConvertHitObjectParser.LegacyHitSampleInfo`.
As long as that state of affairs remains, `HitSampleInfo` must be able
to represent anything that `LegacyHitSampleInfo` can, if feature parity
is to be achieved.
Problem 2: The 0 & 1 sample banks
---
Custom sample banks of 2 and above are a pretty clean affair. They map to
a suffix on the sample filename, and said samples are allowed to be
looked up from the beatmap skin. `Suffix` already exists in
`HitSampleInfo`.
However, the 1 custom sample bank is evil. It uses *non-suffixed*
samples, *allows lookups from the beatmap skins*, contrary to no bank /
bank 0, which *also* uses non-suffixed samples, but *doesn't* allow them
to be looked up from the beatmap skin.
This is why `HitSampleInfo.UseBeatmapSamples` has been called to
existence - without it there is no way to represent the ability of using
or not using the beatmap skin assets.
As has been stated previously in discussions about this feature, it's
both a *mapping* and a *skinning* concern.
There are many things you could do about either of these problems, but I
am pretty sure tackling either one is going to take *many* more lines of
code than this commit does. Which is why this is the starting point of
negotiation.
The equality check that was supposed to replace the read of
`CurrentSelectionItem` showed up as a hotspot in profiling.
The selection updating code looks a little stupid after this, but all in
the name of performance...
Beatmap panels can be visible for very brief instants.
`PanelSetBackground` has a backstop to prevent expensive background
loads which is based on the position of the panel relative to centre of
screen.
However, retrieving the working beatmap that *precedes* any of that
expensive background load logic, is *also* expensive, and *always* runs
even if a panel is visible on screen for only a brief second. Therefore,
by moving some of that background load delay towards delaying retrieving
the working beatmap, we can save on doing even more work, which has
beneficial implications for performance.
I had previously made it invariant in
https://github.com/ppy/osu/pull/32837, and in another instance of past
me being an asshole, I can't actually find the reasoning for this at
this time.
That said, you'd be excused for thinking "why does this matter"? Well,
this will fix https://github.com/ppy/osu/issues/35381, because that
failure only occurs when the user's culture is set to one that doesn't
use a decimal point (.) but rather a decimal comma (,). This messes with
framework, which uses the *current* culture to check for decimal
separator rather than invariant:
d3226a7842/osu.Framework/Graphics/UserInterface/TextBox.cs (L106-L111)
An alternative would be to change framework instead to always accept the
invariant decimal separator.
God I hate this culture crap.
* SSV2 : Replace Mark as Played with Remove from Played if already played
* Remove checks of BeatmapInfo.LastPlayed for DateTimeOffset.MinValue
* Make FooterButtonOptions use a RealmLive<BeatmapInfo> and act on review comments
* FIXUP: Detach BeatmapInfo before passing it to FooterButtonOptions.Popover
---------
Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
* Remove unnecessary information from matchmaking beatmap panel
* Move avatar overlay inside card for better layout
* Allow higher jumping when jumping in succession
* Exclude player panel avatars from masking
* Adjust player panel animations a bit further
* Add avatar-only display mode
* Fix round warmup test not working
* Remove dead test scenes
* Fix edge case where users are added to not-yet-loaded card
* Decouple `PlayerPanel` from `UserPanel`
* Fix remaining test failure (and rename test to match new naming)
From local testing on release build (such that online beatmaps are
accessible) with a large database it seems that maybe this'll help with
recurrent complaints of 'stutters'.
Co-authored-by: Dean Herbert <pe@ppy.sh>
I'm not exactly sure on the reproduction scenario here, but I have had
switching ruleset with converts disabled crash on me a few times
today. It appears to happen sometimes when after the switch the expanded
group no longer exists in the set mapping, because a filter just ran and
removed that group from set of possible groups because there'd be no
beatmaps under it.
I tried to manufacture a quick test but it's not a quick one to test
because filtering intereference is required to reproduce, I think.
I will try again on request but I mostly just want to get this fix out
ASAP before I finish up for the day.
Because the detached store exists and has a chance to actually
semi-reliably intercept a beatmap update operation, I decided to try
this. It still uses a bit of a heuristic in that it checks for
transactions that delete and insert one beatmap each, but probably the
best effort thus far?
Notably old song select that was already doing the same thing locally to
itself got a bit broken by this, but with some tweaking that *looks* to
be more or less harmless I managed to get it unbroken. I'm not too
concerned about old song select, mind, mostly just want to keep it
*vaguely* working if I can help it.
Addresses https://github.com/ppy/osu/discussions/35182.
The source as it is would have you believe that this is correct and
intentional but I'm not so sure about that. For one thing, I
cross-checked stable, and sure, missing tiny droplets does change the
state to fail over there. For another, the guard in master at
5e642cbce7/osu.Game.Rulesets.Catch/UI/Catcher.cs (L250)
is very suspicious, given that it is dead in cause of tiny droplets
because of a preceding guard:
5e642cbce7/osu.Game.Rulesets.Catch/UI/Catcher.cs (L227-L228)
Looking into blame, the tiny droplet guard originates at
e7f1f0f38b, wherein it looks to have been
aimed at handling *hyperdash* state specifically. Later, the logic has
been moved around and around like five times; at
7069cef9ce, the catcher animation logic
was added *below* the hyperdash-aimed guard without the comment being
updated in any way; 5a5c956ced moved the
logic from `CatcherArea` to `Catcher`, while simultaneously changing
the inline comment to no longer mention hyperdashing; and finally,
1d669cf65e added specific testing of tiny
droplets not changing catcher animation state, which I wager to be
back-engineered from the implementation as-it-was rather than
supported by any actual ground truth.
For additional reference:
- catcher animation logic in stable is at 67795dba3c/osu!/GameModes/Play/Rulesets/Fruits/RulesetFruits.cs#L411-L463
- hyperdash application logic in stable is at 67795dba3c/osu!/GameModes/Play/Rulesets/Fruits/RulesetFruits.cs#L165-L171
Addresses https://github.com/ppy/osu/issues/33443, maybe.
I considered adding tests but they'd likely be janky and take a long
time to write, so decided against until there's a demand for it.
Closes https://github.com/ppy/osu/issues/34062.
The root cause of the issue is that `OnEntering()` calls
`onArrivingAtScreen()`, which calls `ensureGlobalBeatmapValid()`, which
would call `checkBeatmapValidForSelection()` with a `FilterCriteria`
instance retrieved from the `FilterControl`.
The problem with that is at the time that this call chain is happening,
`FilterControl` is not yet loaded, which means in particular that it has
not bound itself to the config bindable, as that happens on
`LoadComplete()`:
bff07010d1/osu.Game/Screens/SelectV2/FilterControl.cs (L198)
To resolve this, retrieve the bindable in `SongSelect` itself, which
ensures it is valid for reading at the time the above call chain
happens.
Because it is 99% sure that doing so will fail and spam the user with
"this beatmap doesn't match the online version" notifications, and
because the map status is "locally modified", they should be pretty
aware of that already. This fixes the primary mode of the failure that
https://github.com/ppy/osu/pull/35173 was attempting to hack around.
This will have regressed somewhere around the time that BSS went live,
because at that point the editor stopped resetting online IDs for
beatmaps that got locally modified, making the `beatmapId <= 0` guards
no longer prevent attempts of submission.
A relatively recent regression. It's maybe not a huge one, in that it
probably doesn't matter all that much, but it is somewhat important to
keep the "locally modified" status of the set for as long as possible.
One reason for that is that keeping the "locally modified" status will
pull up a dialog when the user attempts to update the beatmap, warning
them that they will lose their local changes - this dialog will not show
if the online lookup flow is allowed to overwrite the map status with
something else.
Closes https://github.com/ppy/osu/issues/34810.
The reason why I touched it in this direction and not the other is only
because the standalone panel positioning of the button was touched last
in 92ed964627, thus I changed the set
panel to match that.
Closes https://github.com/ppy/osu/issues/35113.
Regressed in dfed564bda - setting
`Carousel.CurrentSelection` was not all that
`requestRecommendedSelection()` was doing there...
A potential point of discussion is whether this global beatmap switch
should be debounced or instant. I'm not sure I have a particularly
well-formed opinion on that. One argument in favour of not debouncing is
that if you look closely at the left side of the screen while the
debounce is in progress, you can still sort of see the broken behaviour
happen - it just doesn't stay there forever.
Thankfully `ensureGlobalBeatmapValid()` being called in every scenario
on screen suspension prevented this bug from being any worse than it is
right now.
Closes https://github.com/ppy/osu/issues/35010.
The issue here does not reproduce consistently, and is more or less
random in presentation. That said, using a large enough realm database
more or less ensures that the issue will present itself (in testing on a
large realm db, the failure rate is around ~50%).
This actually regressed in https://github.com/ppy/osu/pull/34842. The
core failure in this case is here:
fd412618db/osu.Game/Screens/SelectV2/BeatmapCarousel.cs (L161)
The `CheckModelEquality()` call above is comparing two `BeatmapInfo`s,
but a84c364e44 changed the
`BeatmapInfo`-comparing path of `CheckModelEquality()` to use
`GroupedBeatmap` instead. Due to this, `CheckModelEquality()` falls back
to reference equality comparison for `BeatmapInfo`s. When that reference
comparison fails, the carousel stops detecting that the current
selection was deleted from under it correctly, and therefore the
proximity-based selection logic never runs.
Due to the human-obvious mechanism of failure and relatively easy
manual reproduction I've decided not to try and add tests for this,
as they are likely to take a long time to write due to the mechanism
of failure being incorrect use of reference equality specifically. That
said, I can try on request.
Probably closes https://github.com/ppy/osu/issues/35138. I'm not sure. I
only got the issue to reproduce once, on dev, using a very large
archive that was uploading really slowly, and then never again.
The working theory is that basically handling of `progressSampleChannel`
is quite dodgy and it could possibly, in circumstances unknown, be
allowed to play forevermore after transitioning to failed / canceled
state. Success state does not get this treatment because it has special
logic to set progress to 1.
11 is excessive. There can ever be at most 9 of these panels, ever,
because there are at most 9 possible letter grades at this time (F, D,
C, B, A, S, S+, SS, SS+).
Easiest way to make this work without rewriting the layout logic.
I think it makes sense to have the button still exist there but not be
usable on certain screens.
* Change maximum UR estimation + buff rhythm
* Penalty for classic ezhd
* Buff mono bonus to counterbalance logic fix
* New miss penalty + slightly nerf length bonus
* Adjust rhythm values
* Adjust penalty and buff high SR acc
* Exclude HDFL from hidden reading penalties
* Make comment a lil nicer
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
Closes https://github.com/ppy/osu/issues/34959.
`ArgonCounterTextComponent` is pretty terrible and prevents being able
to fix the issue easily. The core issue is that this is the first
instance of the component's usage where the label text can be longer
than the counter in the X dimension, so the total width of any counter
is equal to max(label width, counter width), and the label will be
aligned to the left of that width, while the counter will be aligned to
the right of that width.
The fix sort of relies on the fact that I don't expect *any* consumer of
`ArgonCounterTextComponent` that meaningfully uses the wireframe digits
to want the non-wireframe digits to be aligned to the *left* rather than
the right. It's not what I'd expect any segmented display to work.
(There are usages that specify `TopLeft` anchor, but they usually
display the same number of wireframe and non-wireframe digits, so for
them it doesn't really matter if the digits are left-aligned to the
wireframes or not.)
Regressed with https://github.com/ppy/osu-server-spectator/pull/311.
As it turns out, the method not just resets ready states but also the
beatmap availabilities. So we have to send it again here.
I have a feeling this is also broken in standard multiplayer in some way
or another.
* Prevent Taiko difficulty crash if a map only contains 0-strains
* Add second check for safety
This is accessing a different array of strains. I'd rather be safe than sorry.
* Add guard in PP too
* Make `MarginOfError` a const
This isn't a super common mod compared to every other one on the list, it's probably not worth the storage (and memory in case of stable) implications. We can look at revisiting this once we have actual spinner difficulty considerations
* Test theory crafting
* Place in more appropriate place
* fix a bit better
* Move things around
* Reduce diff
---------
Co-authored-by: StanR <hi@stanr.info>
* New acc curve
* Penalise rhythm difficulty based on unstable rate
* Rename mono acc stuff for more clarity
* Fix nullable
* Rename stuff
* Get actual estimation for SS unstable rate
* Double space my bad
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Calculate consistency factor from object strains
* Use `totalDifficultHits` in performance calc
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Rebalance aim and speed
* Rebalance star rating
* Attempt further speed balancing
* More balancing
* More balancing
* Buff aim a bit
* More speed balancing
* Global rebalance
* Speed balancing
* Global rebalancing
* More speed balancing
* Buff aim
* MORE BALANCING
* Revert "Rebalance star rating"
This reverts commit f48c7445e12174c65b74edfef863cb3ae3cc29ff.
* Decouple velocity change bonus from wide angle bonus
* Replace sin with smoothstep
* Set multiplier back to 0.75
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Change pp summing and adjust multipliers
* Add back convert consideration for hidden
* And the other one whoops
---------
Co-authored-by: StanR <hi@stanr.info>
* Reduce combo scaling for osu!catch
This is a conservative reduction, a middle point between the current
scaling and the CSR proposals.
* Reduce osu!catch combo scaling further
0.45 makes little difference so let's reduce it a bit more.
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* initial commit
* changed HD curve
* removed AR variable
* update for new rework
* nerf HD acc bonus for AR>10
* add another HD nerf for AR>10
* Update OsuDifficultyCalculator.cs
* fix speed part being missing
* Update OsuDifficultyCalculator.cs
* rework to difficulty-based high AR nerf
* move TC back to perfcalc
* fix nvicka
* fix comment
* use utils function instead of manual one
* Clean up
* Use "visibility" term instead
* Store `mechanicalDifficultyRating` field
* Rename `isFullyHidden` to `isAlwaysPartiallyVisible` and clarify intent
* Remove redundant comment
* Add `calculateDifficultyRating` method
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Buff precision difficulty rating in osu!
* Fix position repetition calculation
* Fix aim evaluator crashing, move small circle bonus calculation, adjust the curve slightly
* Refactor
* Fix code quality
* Semicolon
* Apply small circle bonus to speed too
* Fix formatting
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* implement stuff
* fix basic issues
* rework calculations
* sanity check
* don't use score based misscount if no scorev1 present
* Update OsuPerformanceCalculator.cs
* update misscount diff attribute names
* add raw score misscount attribute
* introduce more reasonable high bound for misscount
* code quality changes
* Fix osu!catch SR buzz slider detection (#32412)
* Use `normalized_hitobject_radius` during osu!catch buzz slider detection
Currently the algorithm considers some buzz sliders as standstills when
in reality they require movement. This happens because `HalfCatcherWidth`
isn't normalized while `exactDistanceMoved` is, leading to an inaccurate
comparison.
`normalized_hitobject_radius` is the normalized value of `HalfCatcherWidth`
and replacing one with the other fixes the problem.
* Rename `normalized_hitobject_radius` to `normalized_half_catcher_width`
The current name is confusing because hit objects have no radius in the
context of osu!catch difficulty calculation. The new name conveys the
actual purpose of the value.
* Only set `normalized_half_catcher_width` in `CatchDifficultyHitObject`
Prevents potential bugs if the value were to be changed in one of the
classes but not in both.
* Use `CatchDifficultyHitObject.NORMALIZED_HALF_CATCHER_WIDTH` directly
Requested during code review.
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Move osu!catch movement diffcalc to an evaluator (#32655)
* Move osu!catch movement state into `CatchDifficultyHitObject`
In order to port `Movement` to an evaluator, the state has to be either
moved elsewhere or calculated inside the evaluator. The latter requires
backtracking for every hit object, which in the worst case is continued
until the beginning of the map is reached. Limiting backtracking can
lead to difficulty value changes.
Thus, the first option was chosen for its simplicity.
* Move osu!catch movement difficulty calculation to an evaluator
Makes the code more in line with the other game modes.
* Add documentation for `CatchDifficultyHitObject` fields
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Move all score-independent bonuses into star rating (#31351)
* basis refactor to allow for more complex SR calculations
* move all possible bonuses into star rating
* decrease star rating scaling to account for overall gains
* add extra FL guard for safety
* move star rating multiplier into a constant
* Reorganise some things
* Add HD and SO to difficulty adjustment mods
* Move non-legacy mod multipliers back to PP
* Some merge fixes
* Fix application of flashlight rating multiplier
* Fix Hidden bonuses being applied when Blinds mod is in use
* Move part of speed OD scaling into difficulty
* Move length bonus back to PP
* Remove blinds special case
* Revert star rating multiplier decrease
* More balancing
---------
Co-authored-by: StanR <hi@stanr.info>
* Add diffcalc considerations for Magnetised mod (#33004)
* Add diffcalc considerations for Magnetised mod
* Make speed reduction scale with power too
* cleaning up
* Update OsuPerformanceCalculator.cs
* Update OsuPerformanceCalculator.cs
* add new check to avoid overestimation
* fix code style
* fix nvicka
* add database attributes
* Refactor
* Rename `Working` to `WorkingBeatmap`
* Remove redundant condition
* Remove useless variable
* Remove `get` wording
* Rename `calculateScoreAtCombo`
* Remove redundant operator
* Add comments to explain how score-based miss count derivations work
* Remove redundant `decimal` calculations
* use static method to improve performance
* move stuff around for readability
* move logic into helper class
* fix the bug
* Delete OsuLegacyScoreProcessor.cs
* Delete ILegacyScoreProcessor.cs
* revert static method for multiplier
* use only basic combo score attribute
* Clean-up
* Remove unused param
* Update osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs
Co-authored-by: StanR <castl@inbox.ru>
* rename variables
* Add `LegacyScoreUtils`
* Add fail safe
* Move `countMiss`
* Better explain `CalculateRelevantScoreComboPerObject`
* Add `OsuLegacyScoreMissCalculator`
* Move `CalculateScoreAtCombo` and `CalculateRelevantScoreComboPerObject`
* Remove unused variables
* Move `GetLegacyScoreMultiplier`
* Add `estimated` wording
---------
Co-authored-by: wulpine <wulpine@proton.me>
Co-authored-by: James Wilson <tsunyoku@gmail.com>
Co-authored-by: StanR <hi@stanr.info>
Co-authored-by: StanR <castl@inbox.ru>
* scale misscount by proportion of difficult sliders
* cap sliderbreak count at count100 + count50
* use countMiss instead of effectiveMissCount as the base for sliderbreaks
* make code inspector happy + cleanup
* refactor to remove unnecesary calculation and need for new tuple
* scale sliderbreaks with combo
* use aimNoSliders for sliderbreak factor
* code cleanup
* make inspect code happy
* use diffcalcutils
* fix errors (oops)
* scaling changes
* fix div by zeros
* Fix compilation error
* Add online attributes for new difficulty attributes
* Formatting
* Rebase fixes
* Make `CountTopWeightedSliders` to remove weird protected `SliderStrains` list
* Prevent top weighted slider factor from being Infinity
---------
Co-authored-by: tsunyoku <tsunyoku@gmail.com>
* basis refactor to allow for more complex SR calculations
* move all possible bonuses into star rating
* decrease star rating scaling to account for overall gains
* add extra FL guard for safety
* move star rating multiplier into a constant
* Reorganise some things
* Add HD and SO to difficulty adjustment mods
* Move non-legacy mod multipliers back to PP
* Some merge fixes
* Fix application of flashlight rating multiplier
* Fix Hidden bonuses being applied when Blinds mod is in use
* Move part of speed OD scaling into difficulty
* Move length bonus back to PP
* Remove blinds special case
* Revert star rating multiplier decrease
* More balancing
---------
Co-authored-by: StanR <hi@stanr.info>
* Move osu!catch movement state into `CatchDifficultyHitObject`
In order to port `Movement` to an evaluator, the state has to be either
moved elsewhere or calculated inside the evaluator. The latter requires
backtracking for every hit object, which in the worst case is continued
until the beginning of the map is reached. Limiting backtracking can
lead to difficulty value changes.
Thus, the first option was chosen for its simplicity.
* Move osu!catch movement difficulty calculation to an evaluator
Makes the code more in line with the other game modes.
* Add documentation for `CatchDifficultyHitObject` fields
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Move difficulty calculation fields from `Slider` to `OsuDifficultyHitObject`
* Remove redundant check
* Use `LastObject` where possible
* Update tests
* Make `LazyTravelDistance` `double`
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
* Use `normalized_hitobject_radius` during osu!catch buzz slider detection
Currently the algorithm considers some buzz sliders as standstills when
in reality they require movement. This happens because `HalfCatcherWidth`
isn't normalized while `exactDistanceMoved` is, leading to an inaccurate
comparison.
`normalized_hitobject_radius` is the normalized value of `HalfCatcherWidth`
and replacing one with the other fixes the problem.
* Rename `normalized_hitobject_radius` to `normalized_half_catcher_width`
The current name is confusing because hit objects have no radius in the
context of osu!catch difficulty calculation. The new name conveys the
actual purpose of the value.
* Only set `normalized_half_catcher_width` in `CatchDifficultyHitObject`
Prevents potential bugs if the value were to be changed in one of the
classes but not in both.
* Use `CatchDifficultyHitObject.NORMALIZED_HALF_CATCHER_WIDTH` directly
Requested during code review.
---------
Co-authored-by: James Wilson <tsunyoku@gmail.com>
# temporary workaround for https://youtrack.jetbrains.com/issue/RIDER-130051/Cannot-resolve-symbol-inspections-incorrectly-firing-for-xmldoc-protected-member-references
# temporary workaround for https://youtrack.jetbrains.com/issue/RIDER-130381/Rider-does-not-respect-propagated-NoWarn-CS1591?backToIssues=false
dotnet_diagnostic.CS1591.severity=none
#license header
file_header_template=Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.\nSee the LICENCE file in the repository root for full licence text.
about:Got something you think should change or be added? Search for or start a new discussion!
- name:osu!stable issues
url:https://github.com/ppy/osu-stable-issues
about:For osu!(stable) - ie. the current "live" game version, check out the dedicated repository. Note that this is for serious bug reports only, not tech support.
url:https://t.me/jvnkosu_chat
about:Your jvnkosu! is not working right? Please contact us using our Telegram chat
@@ -73,6 +73,9 @@ Aside from the above, below is a brief checklist of things to watch out when you
After you're done with your changes and you wish to open the PR, please observe the following recommendations:
- Please submit the pull request from a [topic branch](https://git-scm.com/book/en/v2/Git-Branching-Branching-Workflows#_topic_branch) (not `master`), and keep the *Allow edits from maintainers* check box selected, so that we can push fixes to your PR if necessary.
- Please pick the following target branch for your pull request:
-`pp-dev`, if the change impacts star rating or performance points calculations for any of the rulesets,
-`master`, otherwise.
- Please avoid pushing untested or incomplete code.
- Please do not force-push or rebase unless we ask you to.
- Please do not merge `master` continually if there are no conflicts to resolve. We will do this for you when the change is ready for merge.
// While .NET 8 only supports Windows 10 and above, running on Windows 7/8.1 may still work. We are limited by realm currently, as they choose to only support 8.1 and higher.
// See https://www.mongodb.com/docs/realm/sdk/dotnet/compatibility/
*Math.Pow((Math.Min(catchCurrent.StrainTime*catcherSpeedMultiplier,265)/265),1.5);// Edge Dashes are easier at lower ms values
}
// There is an edge case where horizontal back and forth sliders create "buzz" patterns which are repeated "movements" with a distance lower than
// the platter's width but high enough to be considered a movement due to the absolute_player_positioning_error and NORMALIZED_HALF_CATCHER_WIDTH offsets
// We are detecting this exact scenario. The first back and forth is counted but all subsequent ones are nullified.
// To achieve that, we need to store the exact distances (distance ignoring absolute_player_positioning_error and NORMALIZED_HALF_CATCHER_WIDTH)
/// Normalized position of <see cref="BaseObject"/>.
/// </summary>
publicreadonlyfloatNormalizedPosition;
/// <summary>
/// Normalized position of <see cref="LastObject"/>.
/// </summary>
publicreadonlyfloatLastNormalizedPosition;
/// <summary>
/// Normalized position of the player required to catch <see cref="BaseObject"/>, assuming the player moves as little as possible.
/// </summary>
publicfloatPlayerPosition{get;privateset;}
/// <summary>
/// Normalized position of the player after catching <see cref="LastObject"/>.
/// </summary>
publicfloatLastPlayerPosition{get;privateset;}
/// <summary>
/// Normalized distance between <see cref="LastPlayerPosition"/> and <see cref="PlayerPosition"/>.
/// </summary>
/// <remarks>
/// The sign of the value indicates the direction of the movement: negative is left and positive is right.
/// </remarks>
publicfloatDistanceMoved{get;privateset;}
/// <summary>
/// Normalized distance the player has to move from <see cref="LastPlayerPosition"/> in order to catch <see cref="BaseObject"/> at its <see cref="NormalizedPosition"/>.
/// </summary>
/// <remarks>
/// The sign of the value indicates the direction of the movement: negative is left and positive is right.
/// </remarks>
publicfloatExactDistanceMoved{get;privateset;}
/// <summary>
/// Milliseconds elapsed since the start time of the previous <see cref="CatchDifficultyHitObject"/>, with a minimum of 40ms.
*Math.Pow((Math.Min(catchCurrent.StrainTime*catcherSpeedMultiplier,265)/265),1.5);// Edge Dashes are easier at lower ms values
}
// There is an edge case where horizontal back and forth sliders create "buzz" patterns which are repeated "movements" with a distance lower than
// the platter's width but high enough to be considered a movement due to the absolute_player_positioning_error and normalized_hitobject_radius offsets
// We are detecting this exact scenario. The first back and forth is counted but all subsequent ones are nullified.
// To achieve that, we need to store the exact distances (distance ignoring absolute_player_positioning_error and normalized_hitobject_radius)
if(Math.Max(osuCurrObj.AdjustedDeltaTime,osuLastObj.AdjustedDeltaTime)<1.25*Math.Min(osuCurrObj.AdjustedDeltaTime,osuLastObj.AdjustedDeltaTime))// If rhythms are the same.
// If objects just go back and forth through a middle point - don't give as much wide bonus
// Use Previous(2) and Previous(0) because angles calculation is done prevprev-prev-curr, so any object's angle's center point is always the previous object
returnMath.Sqrt(4+rhythmComplexitySum*rhythm_overall_multiplier)/2.0;// produces multiplier that can be applied to strain. range [1, infinity) (not really though)
doublerhythmDifficulty=Math.Sqrt(4+rhythmComplexitySum*rhythm_overall_multiplier)/2.0;// produces multiplier that can be applied to strain. range [1, infinity) (not really though)
/// This function is a harsher version of current combo-based miss count, used to provide reasonable value for cases where score-based miss count can't do this.
publicconstdoublePERFORMANCE_BASE_MULTIPLIER=1.15;// This is being adjusted to keep the final pp value scaled around what it used to be when changing things.
publicconstdoublePERFORMANCE_BASE_MULTIPLIER=1.14;// This is being adjusted to keep the final pp value scaled around what it used to be when changing things.
// As we're adding Oks and Mehs to an approximated number of combo breaks the result can be higher than total hits in specific scenarios (which breaks some calculations) so we need to clamp it.
// There is a low probability of extra slider breaks on effective miss counts close to 1, as score based calculations are good at indicating if only a single break occurred.
/// Milliseconds elapsed since the start time of the previous <see cref="OsuDifficultyHitObject"/>, with a minimum of 25ms.
/// <see cref="DifficultyHitObject.DeltaTime"/> capped to a minimum of <see cref="MIN_DELTA_TIME"/>ms.
/// </summary>
publicreadonlydoubleStrainTime;
publicreadonlydoubleAdjustedDeltaTime;
/// <summary>
/// Normalised distance from the "lazy" end position of the previous <see cref="OsuDifficultyHitObject"/> to the start position of this <see cref="OsuDifficultyHitObject"/>.
// There are two types of slider-to-object patterns to consider in order to better approximate the real movement a player will take to jump between the hitobjects.
// this finds the positional delta from the required radius and the current position, and updates the currCursorPosition accordingly, as well as rewarding distance.
// Snapping inherited B-spline control points to nearby objects would be unintuitive, because snapping them does not equate to snapping the interpolated slider path.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.