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

"Improved" InvokeCallbackSafe method to better guard against concurrent modification #1254

Open
wants to merge 1 commit into
base: develop
from

Conversation

@Zeejfps
Copy link

@Zeejfps Zeejfps commented Dec 16, 2020

Modified all InvokeCallbackSafe methods to Clone the InlineArray of callbacks in order to better guard against removal and addition of new callbacks while still inside the loop.

Currently, when a user registers a new callback, inside another callback, the callback gets executed immediately. I believe this is counter intuitive to what a user might expect.

For example, when an input action is triggered a game object is enabled. Inside that gameobject OnEnabled is called. Say I want to subscribe to the same input action that triggered this. However, as soon as I subscribe to the input action my method will be executed before even Start is called.

One possible solution could have been to iterate over the callback array backwards, however, this wouldn't account for callbacks being removed and added while still in the loop.

My solution is to use the Clone method of the InlineArray to create a copy of the array we can iterate over to avoid being affected by concurrent modification.

…allbacks in order to better guard agains removal and addition of new callbacks while still inside the loop
@Zeejfps Zeejfps changed the title "Improved" InvokeCallbackSafe method to better guard against removal and addition of new callbacks "Improved" InvokeCallbackSafe method to better guard against concurrent modification Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant
You can’t perform that action at this time.