Initial check-in of bevy_undo.

[viridia]

Objective

Fixes #23354

Solution

A basic undo / redo framework (WiP)

Testing

Unit tests so far.

[github-actions]

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

[andriyDev]

bevy_undo already exists as a crate. Is this an upstreaming? Did we get publish permission for this crate?

[viridia]

bevy_undo already exists as a crate. Is this an upstreaming? Did we get publish permission for this crate?

There is an existing crate, bevy-undo with a dash instead of an underscore. There is no bevy_undo on crates.io. I don't know enough about crates.io to know whether or not this will cause a conflict.

The bevy-undo crate is deprecated, and the link to the repository is a dead link.

[viridia]

OK so it looks like crates.io treats dashes and underscores as equivalent, so there will in fact be a conflict. We have three choices:

• Pick a different name (commence bikeshedding)
• Try to contact the crate owner
• Put the undo code in bevy_ui or somewhere else instead of making it a separate crate.

[andriyDev]

Another day, another "why the heck did they allow dash in crate names if it's not allowed in the language".

I feel strongly that making it a separate crate is the correct move, so my preference is try to contact the crate owner then pick a different name.

If we want to make it fun, I propose bevy_forwardback, or less fun bevy_history. Both of these are much less obvious than "undo" though haha.

[andriyDev]

I don't really like how hyper focused on editing UI elements this is, and I'm not even sure it's sufficient for that.

I've tried implementing my own undo system before, and the problem is always entities. We need a way to map entities before and after an undo. If an action is spawn an entity, then undoing that action should despawn that entity, and redoing that action should spawn a new entity with the same components. However that also means we need to fix all the references in the world we can find.

I don't think this is something we can leave to implementors, or patch on later.

Maybe I'm ballooning the scope here though? idk. If the idea is to never deal with entities directly (e.g., only touch BSN or something), then maybe it'll be fine?

[andriyDev]

I wonder if we should take these as owned, and then have the return type return Result<Box<dyn UndoAction>, (Box<dyn UndoAction>, Box<dyn UndoAction>)>. It is definitely a more complex signature, but consider that the contents of these undo actions might be "every component that included a particular Entity" since we need to rename the Entity.

[andriyDev]

I suspect this is not sufficient. For example, if your undo action spawns an entity (you previously despawned the entity), you also need to modify every other undo action to point at the new entity, else the references will be broken.

[viridia]

Hmmm. I haven't run into this problem before, because all my editors edit assets, not entities (although some assets produce entities when loaded, the entities are not the source of truth). Still, worth thinking about.

Can you talk more about your previous experience? What kinds of entities were you undoing?

[andriyDev]

I was trying to make a generic undo/redo system (which ideally would be the case for a Bevy official undo/redo system). So it could be any kind of entity, including one with references.

In particular, I was trying to make an editor whose workflow was: spawn entities, add components to those entities, and then serialize those entities. Loading the editor would just mean deserializing those entities and spawning them. But that also means the entities are effectively the SOT.

[github-actions]

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

[niklasBenjamin]

Shouldn't we apply the action to the world first, then push the inverse onto the undo stack?

[viridia]

Depends on whether you are using snapshot or delta style. For a snapshot, you'll want to push the current undo state first.

[niklasBenjamin]

That makes sense! Thanks for the clarification