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

Add daily challenge animation to daily challenge notification #29770

Merged
merged 4 commits into from
Sep 8, 2024

Conversation

Ianlucht
Copy link
Contributor

@Ianlucht Ianlucht commented Sep 7, 2024

Added if conditional similar to main menu if conditional to activate DailyChallengeNotification if the user opens the Daily Challenge from the notification. Potentially excessively verbose, however this is future proof in the scenario that the Daily challenge is opened from the menu and then opened again from the notification menu.

resolves #29747

@Ianlucht Ianlucht changed the title Add animation to challenge notification Add daily challenge animation to daily challenge notification Sep 7, 2024
@bdach bdach self-requested a review September 8, 2024 10:56
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.

I am choosing to ignore the code duplication, because it's non-trivial duplication of trivial logic (see PerformFromScreen() usage that isn't there in the other place) and there are only 2 places that do this for now. If this changes/happens again and/or there are more places that do this in the future then splitting a helper should be reconsidered.

@bdach bdach merged commit 139fc07 into ppy:master Sep 8, 2024
13 checks passed
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.

Open Daily Challenge from Notification will not play the Intro Animation
3 participants