Skip to content

Fixed handleTutorial double invocation#73

Open
MarvelProgramming wants to merge 1 commit into
mainfrom
handle-tutorial-double-invocation-fix
Open

Fixed handleTutorial double invocation#73
MarvelProgramming wants to merge 1 commit into
mainfrom
handle-tutorial-double-invocation-fix

Conversation

@MarvelProgramming
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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