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

Fixed handleTutorial double invocation #73

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MarvelProgramming
Copy link
Contributor

@MarvelProgramming MarvelProgramming commented Oct 12, 2022

-Put handleTutorial invocation behind a check for the "mapDidLoad" event

Reasoning: While working on animation cancelling, this "double invocation" would cause unexpected behavior due to several duplicate async functions being called

-Put handleTutorial invocation behind a check for the "mapDidLoad" event
@rmkubik
Copy link
Contributor

rmkubik commented Oct 13, 2022

Does this correspond to an open issue?

What does putting handleTutorial behind mapDidLoad change or fix?

@MarvelProgramming
Copy link
Contributor Author

MarvelProgramming commented Oct 13, 2022

Does this correspond to an open issue?

What does putting handleTutorial behind mapDidLoad change or fix?

This was something I encountered while working on animation cancelling. Since handleTutorial was being invoked twice (once on "mapDidLoad" and again on "levelDidLoad"), it would cause the cancellation to behave in unexpected ways. I'll add this to the initial comment, since this PR seems kind of random otherwise haha

@MarvelProgramming MarvelProgramming self-assigned this Oct 21, 2022
@MarvelProgramming MarvelProgramming added the bug Something isn't working label Oct 21, 2022
@rmkubik
Copy link
Contributor

rmkubik commented Nov 8, 2022

I know we did a review of animation cancelling and did some feedback on it.

Is this PR still relevant in light of that review?

@MarvelProgramming
Copy link
Contributor Author

Is this PR still relevant in light of that review?

It's hard to say. Depending on how we implement animation cancelling (even with the event based approach), this may still cause unintended behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants