Avoid triggering `Added` and `Changed` queries on all rollbacks?

[johanhelsing]

When a rollback is triggered, bevy_ggrs restores a saved world snapshot. It updates components by first removing them, and then adding them again. This triggers all queries with Added<Component> or Changed<Component> in it, regardless of whether the component was already there and whether it changed.

https://github.com/gschup/bevy_ggrs/blob/b730a0b1a765bca2cb79466c36d86cac08ae1a12/src/world_snapshot.rs#L177

Sometimes systems have queries that react to other components being added or changed, triggering changes on every rollback. Sometimes, this just causes a small performance hit, other times, it leads to buggy behavior because setup code runs multiple times.

Describe the solution you'd like Perhaps it's possible to only update the component if it's different?

Additional context In regular game code, this is usually quite easy to fix or work around, but sometimes the affected code belongs to 3rd party crates, which are a lot harder to solve.

[zicklag]

This one could be quite tricky, and maybe infeasible.

This is a similar issue to using Local<T> parameters in your systems, because the local system state is what stores the last change tick if I remember correctly, so we'd essentially have to have a way to solve snapshot and restore for local state.

---

Here's one idea. We'd have to make a trait that adds a .rollback_system() method that you could call when adding systems to the app, and this would wrap around a normal system implementation, and it would use a snapshotted resource to store the SystemState for all rollback systems.

I think that could fix the Local<T> and other local state parameters, and it could even fix event readers ( if you custom register the events and make sure you only flush the event buffer on confirmed frames ).

But we'd also have to have a way to revert the latest change tick on a component ( which is not a part of local state ) when we restore an older snapshot. I don't know if Bevy has a publicly exposed way to mutate those component change ticks, though.

[johanhelsing]

I looked into this issue and wanted to understand exactly what was wrong with doing ReflectComponent::apply. The details of that are explained in the bevy issue above.

The problem is not with bevy_reflect::Array + !bevy_reflect::Lists (which are fixed size and will be applied in their entirety), but with bevy_reflect::Lists which is confusingly a supertrait of Array.

For instance, the Children component uses a SmallVec, which is a List, hence the issues with entities not being removed if self.len() > value.len().

There is a pr for adding remove functionality to List, so perhaps there is hope for this to work after all :) https://github.com/bevyengine/bevy/pull/7063

[johanhelsing]

Relevant bevy pr implementing change detection bypass: https://github.com/bevyengine/bevy/pull/5635

[gschup]

Since the bevy PR is now merged, we should be able to use it now, right?

[johanhelsing]

If i remember correctly, we can't do the nicest fix, since reflect apply is imo still broken. But perhaps we might get a decent workaround with change detection bypass.

This will probably all be irrelevant after the rewrite, though?