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

Undo / Redo feature #2

Open
vincentfretin opened this issue Jul 14, 2023 · 9 comments
Open

Undo / Redo feature #2

vincentfretin opened this issue Jul 14, 2023 · 9 comments
Assignees
Labels
enhancement New feature or request sponsors needed

Comments

@vincentfretin
Copy link
Member

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 aframevr#21

@vincentfretin
Copy link
Member Author

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
Copy link
Member Author

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
Copy link

kfarr commented Jun 10, 2024

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
Copy link

dmarcos commented Jun 11, 2024

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

@vincentfretin
Copy link
Member Author

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
Copy link

dmarcos commented Jun 12, 2024

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

@vincentfretin
Copy link
Member Author

@dmarcos see 3DStreet/3dstreet#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
Copy link

dmarcos commented Jul 1, 2024

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
Copy link
Member Author

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 aframevr#523 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request sponsors needed
Projects
Status: Todo
Development

No branches or pull requests

3 participants