-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix transition panic while currently in transition #8
Conversation
Forgot that it also needs to fix the director panic, getting that now |
29c0fe2
to
e09482a
Compare
Great, thank you for the fix. I'm writing an acknowledgment so people know that the expected behaviour is to cancel the current transition. I prefer to simplify the conditions so everytime a switch or trigger is processed while a transition is running cancels the transiton. I think that cancelling the transition is better because if you need to prevent this behaviour you can prevent switches and triggers to occur using using a transitionAware scene. But I'd like to know why did you add the return on the ProcessTrigger, so I'm not missing anything. |
The PR as currently written will not just end the current transition but skip on to the transition of the next scene (as if you had clicked to start that next scene after waiting for the transition to finish). However, to answer your question about the return in SceneDirector.ProcessTrigger was because without the early return it would just end up replaying the same transition over, and I'm not sure why just that playing with it and putting a return in made it behave like the SceneManager.SwitchWithTransition I just described rather than replaying it. |
Strange. I'll create some tests and investigate further. I'm afraid that any losing ends on that part could kill the library for large projects. Luckily you spoted it early. Thank you. |
I should note I was using a modified examples/director where I gave both scenes a transition instead of just the one, that might give you a better idea of what happens when you see it. |
I decided to remove the trigger checking that you added since the I figured that if the ProcessTrigger is called by a different trigger the bug would still occur. Maybe what you found was because of a pointer to the same transitions or scene, idk, I couldn't reproduce it. I added new tests and they are running fine, checkout if they help you find the problem with the example. If everything is fine we can release the patch. |
- 1st scene: Red, 2nd scene: Blue, 3rd scene Green
Thanks for the updated example. I found the problem and explained better on the revision. To prevent this double call I added some conditions to the example. Now it seens to work as expected, a double click will bring you to the ThirdScene instead of the FirstScene. |
Fixes issue #7