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

Implement undo system (WIP) #115

Merged
merged 5 commits into from
Nov 19, 2023
Merged

Implement undo system (WIP) #115

merged 5 commits into from
Nov 19, 2023

Conversation

perkele1989
Copy link
Contributor

@perkele1989 perkele1989 commented Oct 22, 2023

This PR is still a work in progress and shouldn't be merged yet.

Implements the foundation for an undo system. Have tried to follow existing codebase style and idioms.

Needs to fix before merge:

  1. Improve memory management on the undo/redo buffer, likely just change it to a preallocated array with a cursor to signify where in commit history we are.
  2. Fix input issue regarding ImGuizmo (inconsistent results from manipulate function)
  3. Implement and invoke initialize/shutdown functions, to clean up memory from the undo/redo stacks

@perkele1989
Copy link
Contributor Author

perkele1989 commented Oct 22, 2023

The gist of it:

  1. Spartan::CommandStack is our "singleton" class to manage the undo stack. It manages the undo/redo buffer, and exposes functions to Undo, Redo, and apply new commands.
  2. To implement a new command, inherit from Spartan::Command, and override the OnApply and OnRevert functions appropriately (making sure to store any required states as member variables in the constructor). Check out Spartan::TransformEntity for a simple example that transforms an entity.
  3. Added obvious hotkeys in WorldViewer (where the other ones are)

Additional thing to consider: Right now we are using the object ID from the entity to track it. Ideally we likely want to track a more consistent ID / UUID / unique entity name, to support things like undo for entity creation/deletion, and scenarios where entities may load/unload in editor. Could track by weak_ptr or shared_ptr too but that wouldn't solve the aforementioned issue.

@PanosK92
Copy link
Owner

Simple, gets the job done, I like it. I'll talk to you in Discord about the IDs.

@NickPolyder
Copy link
Contributor

Hello,

Can I ask what is left to be done on this PR ?

From what I can understand a basic MVP of the undo system is there. Why not trial it as is and let people come up with ideas on how to make it better or simply make other commands for their favourite features ?

Best,

@perkele1989
Copy link
Contributor Author

Hello,

Can I ask what is left to be done on this PR ?

From what I can understand a basic MVP of the undo system is there. Why not trial it as is and let people come up with ideas on how to make it better or simply make other commands for their favourite features ?

Best,

Hey,

sorry I’ve been swamped and haven’t been able to work much on this, but the idea is to get basic undo functionality going I guess before merging this. I’m working right now on implementing internal serialization to facilitate undoing object deletion and similar.

@NickPolyder
Copy link
Contributor

Oh Cool. Yeah dont worry about the timing everybody have life to deal with.

Keep up the good work.

Best,

@PanosK92
Copy link
Owner

The foundation is there, once functionality of existing commands is glitch free (say transform gizmo manipulation), I can merge this, and we'll take it from there. Thanks!

@ApostolosBouz
Copy link
Contributor

ApostolosBouz commented Nov 19, 2023

(say transform gizmo manipulation).

What's the issue with the transform gizmo again ?

@PanosK92 PanosK92 merged commit f820924 into PanosK92:master Nov 19, 2023
2 checks passed
PanosK92 added a commit that referenced this pull request Jan 12, 2025
Implement undo system (WIP)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants