Skip to content
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

Apply various fixes and cleanup to spinner logic #25143

Merged
merged 9 commits into from Oct 16, 2023

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Oct 16, 2023

This takes a good portion of #24999 which can be immediately merged and adds a bit of clean-up on top.

Of note, the majority of this is @smoogipoo's work. I'll add notes for the changes I've made on top:

0bb95cf Fix incorrect initial rotation transfer value

Should have been removed as part of #24360. As discussed on discord

10bab61 Tidy up lastAngle usage and add assertion of maximum delta

As a precursor to an attempt at refactoring the SpinnerSpinHistory change, I want to assert the maximum input delta to the equation. I also found that lastAngle was being updated in the normalisation logic when it didn't need to be.

04af46b Change SpinFramesGenerator to take degrees as input

We use angles in most other places so this feels more natural to work with.


I've also added Ignore rules to some tests which are still valuable for working forward, but will fail on the current implementation of spinner rotation handling.

@smoogipoo smoogipoo requested a review from bdach October 16, 2023 13:52
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine

@bdach bdach enabled auto-merge October 16, 2023 16:59
@bdach bdach disabled auto-merge October 16, 2023 20:30
@bdach bdach merged commit 240d317 into ppy:master Oct 16, 2023
11 of 17 checks passed
@peppy peppy deleted the split-spinner-fix branch October 17, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants