-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Implement beat-synchronized animation in skip overlay #29866
Conversation
ahh, I didn't mean to merge it directly in the main. It's my first pull request, so sorry. |
More importantly, why is the diff listing 405 files changed? It very much should not. Please force push that away or reopen your change with the proper diff. |
Yeah, I was surprised too. It happened after I ran a code formatter over it, or rather, entered the |
This reverts commit 630b8a6.
Yeah we don't use |
Should not be merged without squash. Also @nekupaw please stop merging master. |
Sorry, I forgot to create a branch at the start. I just intended to include the new merges. |
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.
while addressing this review please fix history as requested by force-pushing (this PR should consist of a single commit)
Anchor = Anchor.BottomCentre, | ||
Origin = Anchor.BottomCentre, | ||
Colour = colours.Yellow, | ||
RelativeSizeAxes = Axes.X, | ||
Masking = true, |
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.
Redundant, Circle
already enables masking by default (it wouldn't be a circle otherwise)
if (fadeOutBeginTime <= gameplayClock.CurrentTime) | ||
return; | ||
|
||
float progress = (float)(gameplayClock.CurrentTime - displayTime) / (float)(fadeOutBeginTime - displayTime); |
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.
Why is fadeOutBeginTime
checked against current time (therefore clamping from the left), but progress is not forcefully clamped to be no larger than 1?
I'll create a pull request using a new branch now, I think this is the easiest way. |
I think the new beat-synced animation in the break overlay is really cool, so I implemented pretty much the same approach in the skip overlay.
I also rounded the corners by replacing the box with a circle.
Let me know what you think.
skipOverlay_beat-synced-animation.mp4