c-frame / aframe-editor

:mag: Visual inspector tool for A-Frame. Hit *<ctrl> + <alt> + i* on any A-Frame scene.
https://aframe.io/aframe-inspector/examples/
MIT License
2 stars 0 forks source link

Undo / Redo feature #2

Open vincentfretin opened 1 year ago

vincentfretin commented 1 year ago

Implement an undo / redo feature.

https://threejs.org/editor/ has an undo/redo feature. Main implementation in this file https://github.com/mrdoob/three.js/blob/master/editor/js/History.js each action is a command https://github.com/mrdoob/three.js/tree/master/editor/js/commands We probably need to hook our events componentadd, componentremove, entityupdate, entityidchange, objectremove, entitycreated, entityclone, etc with a system like that.

Initially discussed in https://github.com/aframevr/aframe-inspector/issues/21

vincentfretin commented 1 year ago

You can sponsor $500 to vincentfretin fully or partially to work on this feature. Find other work you can sponsor at https://github.com/c-frame/sponsorship

vincentfretin commented 1 year ago

This feature could then be backported to aframe-inspector if the changes are not too heavy and @dmarcos agrees to review it and merge there.

kfarr commented 5 months ago

Note: this could be implemented step by step, not all at once, to simply allow undo of certain actions first and then add others later

dmarcos commented 5 months ago

Yeah. We can do it in the inspector if can be implemented simply and reliably

vincentfretin commented 5 months ago

I discussed a bit with @kfarr on the implementation, just writing down what I just said. If you look at for example SetPositionCommand command in threejs editor so https://github.com/mrdoob/three.js/blob/master/editor/js/commands/SetPositionCommand.js and used from transformControls: https://github.com/mrdoob/three.js/blob/4fdc32283d7aa0502889e8bde57221bb4b69feeb/editor/js/Viewport.js#L111 or from the ui: https://github.com/mrdoob/three.js/blob/4fdc32283d7aa0502889e8bde57221bb4b69feeb/editor/js/Menubar.Edit.js#L99

The equivalent of that in aframe inspector is the entityupdate event with component "position" We use entityupdate event in three places https://github.com/aframevr/aframe-inspector/blob/0b8b8d54d967b425d937b81133c8cc340bde1017/src/lib/viewport.js#L80 https://github.com/aframevr/aframe-inspector/blob/0b8b8d54d967b425d937b81133c8cc340bde1017/src/lib/entity.js#L38 https://github.com/aframevr/aframe-inspector/blob/0b8b8d54d967b425d937b81133c8cc340bde1017/src/components/components/Mixins.js#L53

This code where we call setAttribute and emit the entityupdate event https://github.com/aframevr/aframe-inspector/blob/0b8b8d54d967b425d937b81133c8cc340bde1017/src/lib/viewport.js#L78-L85 would need to be rewritten to be something like editor.execute( new EntityUpdateCommand( editor, {component, entity, property, value})) and it's in EntityUpdateCommand.execute that we do the setAttribute and emit entityupdate.

A command may be updatable, see https://github.com/mrdoob/three.js/blob/134ff886792734a75c0a9b30aa816d19270f8526/editor/js/History.js#L54-L58 if last command is of the same type as the previous one the same entity and component during a window of 500ms keep the last one, so that part https://github.com/mrdoob/three.js/blob/134ff886792734a75c0a9b30aa816d19270f8526/editor/js/History.js#L42-L45 that needs to be properly modified.

dmarcos commented 5 months ago

Cool. Thanks for the elaboration. It doesn't look too complicated?

vincentfretin commented 4 months ago

@dmarcos see https://github.com/3DStreet/3dstreet/pull/676 that implements undo position/rotation/scale and any change in a component. Currently mixin is not handled. Are you interested that I do a similar PR to aframe-inspector? 95% of the changes would be the same.

dmarcos commented 4 months ago

Yeah thanks! Question. Would it be possible to have an undo / redo component independent of the inspector? That might be useful for the community

vincentfretin commented 4 months ago

You would still need some changes in the inspector to replace setAttribute+emit('entityupdate') by another event emit('updateentity') that would actually do the setAttribute+emit('entityupdate'). And if you want the undo module out of the core, we need a way to unregister the default updatentity handler and replace by another that uses the history undos/redos stack. For the shortcuts, it seems there was previously a way to create module with some shortcuts registerModuleShortcut https://github.com/aframevr/aframe-inspector/blob/b792e9113c416f758caccf154acf558c6f538e36/src/index.js#L19 https://github.com/aframevr/aframe-inspector/blob/b792e9113c416f758caccf154acf558c6f538e36/src/lib/shortcuts.js#L112-L123 https://github.com/aframevr/aframe-inspector/blob/b792e9113c416f758caccf154acf558c6f538e36/src/lib/shortcuts.js#L183-L216 but not clear how to use that. It seems to be dead code from a modules attempt that was partially removed, see https://github.com/aframevr/aframe-inspector/issues/523#issuecomment-338615746