Fixed concurrency issue with the usage of setState

[NyaStone]

In the update handler for the game engine, the updateHandler that calls the systems, the react component method setState was used to update the state of the react component by referring to the previous state using the state property directly.

This can lead to concurrency issues that could create behaviors like the systems loop using the default state of null from the constructor and then overwriting the initial entities with null. Afterward, it may keep looping over the kept null value.

Instead of passing a value to setstate, this can be prevented by giving it a callback that will receive the latest state as argument. However, to use setState that way, react requires the component to be mounted. So the subscription to the timer had to be moved from constructor to componentDidMount.

[bberak]

Thanks @NyaStone

Is there any inherent risks with moving the the timer subscription from the constructor to componendDidMount?

I'm just thinking about this in terms of incrementing the major or minor version number.

This seems like an appropriate change overall..

[NyaStone]

ComponentDidMount should only get triggered once as the component gets mounted onto screen/DOM. According to the react documentation, it is the perfect place for data fetching, DOM manipulations or subscriptions. https://react.dev/reference/react/Component#componentdidmount

The cleanup/unsubscription was already done within the componentWillUnmount method so there should be no problem with moving subscription to componentDidMount, the only effect is that it prevents the systems loop from starting before the react lifecycle.

[bberak]

That sounds spot on.. I'll merge this PR into master and give it a run using one of my projects. ETA sometime this week. Great PR, thank you 🙏

[bberak]

Thanks for your contribution @NyaStone - I've just published react-game-engine@1.2.0 with your changes 🙏..

I'll probably need to consider migrating this change over to react-native-game-engine too 🤔