-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix swells not being correctly treated in editor gameplay test #28995
Conversation
Closes ppy#28989. Because swell ticks are judged manually by their parenting objects, swell ticks were not given a start time (with the thinking that there isn't really one *to* give). This tripped up the "judge past objects" logic in `EditorPlayer`, since it would enumerate all objects (regardless of nesting) that are prior to current time and mark them as judged. With all swell ticks having the default start time of 0 they would get judged more often than not, leading to behaviour weirdness. To resolve, give swell ticks a *relatively* sane start time equal to the start time of the swell itself.
@@ -11,5 +11,6 @@ | |||
</PropertyGroup> | |||
<ItemGroup Label="Project References"> | |||
<ProjectReference Include="..\osu.Game.Rulesets.Taiko\osu.Game.Rulesets.Taiko.csproj" /> | |||
<ProjectReference Include="..\osu.Game.Tests\osu.Game.Tests.csproj" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional - the added reference allows using BeatmapImportHelper.LoadOszIntoOsu()
in the test, which resides in osu.Game.Tests
.
Yes I considered moving that class to osu.Game
, but then I also have to move its dependency TestResources
, and then I probably need to move all of the resources that references to the main game project, so not doing that.
Yes I considered not relying on an import in the test, and tried making it work with an empty beatmap, and it doesn't work for unknown reasons that I don't want to waste an hour debugging. A non-empty beatmap that hasn't been imported via standard import flows dies on a realm assertion.
So if this is deemed unacceptable then I'll likely just force-push e2fe193 entirely out of this diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with this FWIW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine to me
Closes #28989.
Because swell ticks are judged manually by their parenting objects, swell ticks were not given a start time (with the thinking that there isn't really one to give). This tripped up the "judge past objects" logic in
EditorPlayer
, since it would enumerate all objects (regardless of nesting) that are prior to current time and mark them as judged. With all swell ticks having the default start time of 0 they would get judged more often than not, leading to behaviour weirdness.To resolve, give swell ticks a relatively sane start time equal to the start time of the swell itself.