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

Map Entity IDs When Writing Snapshot to World #31

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

zicklag
Copy link
Contributor

@zicklag zicklag commented Nov 10, 2022

Resolves #29.

I haven't really tested this yet, but this should fix entity mapping and hierarchy problems.

If this works it'd be good to have some documentation on how this effects types that store entities.

For instance, if you create your own Inventory component that needs to be synced, you must implement and reflect MapEntities for it, so that it will get the new entity IDs when the snapshots are applied.

@gschup
Copy link
Owner

gschup commented Nov 13, 2022

This looks lovely, but it would be good if we could see it working before merging. We could simply add a smaller cube as a child on top of the player cubes in the examples. What do you think?

@zicklag
Copy link
Contributor Author

zicklag commented Nov 13, 2022

Yeah, we should definitely test it, but I think it requires a pretty specific test.

I think to run into the issue, we actually need to add a child to an entity, then delete the child in a later frame ( specifically we must use the Children component to decide which entity to delete ), then have it roll back, which will invalidate the entity as it gets re-created during the rollback. Then we just need to make sure that during a sync test the entity gets deleted without being re-created after rolling back and forward.

Actually I think that's isolated enough we can add it as a test case. I'll see if I can put that in a test and verify that it fails before this PR and works after this PR.

@gschup
Copy link
Owner

gschup commented Nov 13, 2022

Perfect!

@zicklag
Copy link
Contributor Author

zicklag commented Nov 13, 2022

There you go! I wrote a test case and discovered a couple bugs which I fixed.

// The user can still register any custom types with `register_rollback_type()`.
r.register::<Parent>();
r.register::<Children>();
r
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to look at this section here. I'm not sure if there's a better place to do this. The comment explains.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense.

@gschup
Copy link
Owner

gschup commented Nov 14, 2022

Awesome work! Thank you so much!

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.

Invalidated Entity's
2 participants