Rename beatmap-level velocity/tick rate to remove "slider" terminology #23432
+148
−94
Conversation
This file contains 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
The intention was to give an idea of what the most common velocity of the beatmap is, but in hindsight, because the "base" velocity is being set elsewhere this doesn't make sense. It will/should be 1.0x. Showing this range is still valuable, though.
I think this makes the most sense of the iterations I've tested so far, albeit maybe being a touch too verbose.
These values are used for more than just sliders across all rulesets, so it makes sense to rename them as such.
1 task
1 task
peppy
added
the
next release
bdach
requested changes
May 8, 2023
Comment on lines
+23
to
+27
| [Ignored] | ||
| public double BaseVelocity { get; set; } = 1; | ||
|
|
||
| [Ignored] | ||
| public double TickRate { get; set; } = 1; |
There was a problem hiding this comment.
As the single test failure demonstrates, ignoring these is running head-first into #20883. These properties will get decoded from the .osu file and then promptly get discarded due to this logic:
osu/osu.Game/Beatmaps/WorkingBeatmap.cs
Lines 230 to 231 in 88135a3
Thus, making it effectively such that every single beatmap ever will have a base velocity and tick rate of 1.
Contrary to #23418, however, I'm not sure how to fix this, because I'm not sure where else these properties would live.
There was a problem hiding this comment.
I guess you could do whatever this hokey-pokey is but it just seems cursed to me...
diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs
index ab790617bb..9f7bf60344 100644
--- a/osu.Game/Beatmaps/WorkingBeatmap.cs
+++ b/osu.Game/Beatmaps/WorkingBeatmap.cs
@@ -227,9 +227,14 @@ private Task<IBeatmap> loadBeatmapAsync()
// The original beatmap version needs to be preserved as the database doesn't contain it
BeatmapInfo.BeatmapVersion = b.BeatmapInfo.BeatmapVersion;
+ var decodedBeatmapInfo = b.BeatmapInfo;
+
// Use the database-backed info for more up-to-date values (beatmap id, ranked status, etc)
b.BeatmapInfo = BeatmapInfo;
+ // but keep the difficulty data I guess??
+ b.BeatmapInfo.Difficulty = decodedBeatmapInfo.Difficulty;
+
return b;
}, loadCancellationSource.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default);
}
osu.Game/Screens/Edit/Compose/Components/Timeline/DifficultyPointPiece.cs
Show resolved
Hide resolved
bdach
removed
the
next release
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.
These values are used for more than just sliders across all rulesets, so it makes sense to rename them as such.
Also removes these values from realm, as they probably shouldn't have been in there in the first place.