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

Move PredictionHistory to the Confirmed entity? #793

Open
cBournhonesque opened this issue Jan 9, 2025 · 4 comments
Open

Move PredictionHistory to the Confirmed entity? #793

cBournhonesque opened this issue Jan 9, 2025 · 4 comments

Comments

@cBournhonesque
Copy link
Owner

Let's say you have a Confirmed entity with a Predicted entity.
The Predicted entity gets despawned on the Client, however we still want to keep the entity around and the PredictionHistory so that we can restore it in case of a rollback.

Current solution: use the prediction_despawn function that just caches the components and then re-adds them. That's annoying because users of the library have to remember to use this special despawn.

New solution: put the PredictionHistory on the confirmed entity. The predicted entity can get deleted; if it gets re-spawned via rollback we just need to copy over the components, we still have access to the history

@OlivierCoue
Copy link
Contributor

I agree, also because prediction_despawn only remove the components which are replicated (registered), not the clients only ones such as sprites.. which you have to manually remove

@cBournhonesque
Copy link
Owner Author

Oh.. but in the proposed option where the predicted entity just uses the normal despawn, you would get the same issue where the sprites, etc. won't be added.

Or maybe we can assume that users have a system/trigger on newly-added predicted to add those extra components?

@cBournhonesque
Copy link
Owner Author

cBournhonesque commented Jan 13, 2025

I would like to think more about this change.

  1. You're right that non-protocol components would not get removed currently, or that child entities do not get affected. That means you could have a sprite/mesh that is still visible at the last predicted position.
    If we change the despawn behaviour to be a despawn_recursive on the client:
  • on rollback, we have to expect that client-specific systems will re-add the mesh/sprites/non-protocol component
  • on rollback, a new Predicted entity would get created. The confirmed/predicted mapping need to be updated. All the protocol components on the confirmed entity would get copied over the predicted entity. But actually to do that there's no need to put the PredictionHistory on the Confirmed component? Since after the predicted entity is despawned we can assume the history is that all components are empty, so on the initial rollback we can just copy all components over from the Confirmed entity and use that to re-create new PredictionHistory
  1. An alternative to actually despawning the entity would be to 'disable' it from queries. There were initiatives in bevy to make an entity 'invisible' to Queries by marking it as disabled. I don't know how this would work with child entities (meshes, etc.) though

  2. Usually we want to trigger some VFX/animation/sound only when we receive a confirmed despawn from the server.
    Otherwise it could be confusing if you do a predicted despawn which is erroneous (rolled back right after) but you still hear a despawn sound or see a death VFX.
    So the users would have to be careful to not trigger those when the Predicted entity is despawned, but instead do it when the Confirmed entity is despawned (but when the predicted component values are still available to know where to play the vfx)

It would be nice to have an example with animation/sounds to show how it would work

@OlivierCoue
Copy link
Contributor

I think we can assume that user will have system that trigger on added predicted entities to add their non-protocol components. This is what you do in most examples and sound like the easier/cleanest pattern. This work for all cases (actual new predicted or predicted added during rollback).

Having to react on removed protocol-only components to also remove non-protocol sounds pretty bad and hard to maintain.

Concerning point 3, while this can be true for critical events I'm not sure we always want to react on confirmed entity despawn, as this could add quite a lot of delay.

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

No branches or pull requests

2 participants