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

Ensure parents are always initialized & started before children #5595

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ElectroJr
Copy link
Member

@ElectroJr ElectroJr commented Jan 8, 2025

This PR makes changes to how the client applies game states to ensure that parent entities are always initialized & started before their children are.

Previously, the client would:

  • Create new uninitialized entities in an undetermined order.
  • Apply all entity states in an undetermined order.
  • Initialize and start entities in an undetermined order (entities are started after all states have been applied).

Now the client will:

  • Create new uninitialized entities in an undetermined order.
  • Sort states to ensure that parents are always processed before children
  • Apply, initialize & start entities sequentially (entities may be started before all states are applied).

The motivation for this change is to fix a bug that can happen if an existing (already initialized) entity gets attached to a newly created (uninitialized) entity. This PR is an alternative to #5588, and also includes a test to check for the bug that it was trying to fix.

One potential issue is that now entities that depend on other entities during their start-up might run into issues due to other entities not necessarily having had their state updated yet, but AFAICT it doesn't cause any issues and all tests seem to pass? And given that component states already need to assume that the states of other entities haven't been applied yet, it seems reasonable to extend that rule to component init & startup logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant