The UndoRedo class has been extended to make it more useful for its own use in addition to internal use by the engine itself. #41193
Conversation
…wn use in addition to internal use by the engine itself.
There was a problem hiding this comment.
set_max_actions/get_max_actions should become a property, using the ADD_PROPERTY macro; you can check how it is used in other classes such as Node.
get_current_action could also be a property (without a setter), though get_action_count should likely not be one.
| <return type="int"> | ||
| </return> | ||
| <description> | ||
| Gets the ammount of actions. |
There was a problem hiding this comment.
Typo: "amount"
Might be nice to clarify what this means exactly, i.e. that it represents the amount of actions currently stored in the undo/redo.
| <argument index="0" name="max_actions" type="int"> | ||
| </argument> | ||
| <description> | ||
| Sets the max ammount of actions. If [code]max_actions[/code] is [code]-1[/code], there is no limit of actions. |
There was a problem hiding this comment.
As above, "amount"
There should be an explanation of what happens after the limit is reached, likely that the oldest action (index 0) is forgotten by the UndoRedo. Also, would be ideal if what happens to action indices after the limit is reached is also clarified.
| @@ -256,6 +256,13 @@ void UndoRedo::commit_action() { | |||
| committing++; | |||
| redo(); // perform action | |||
| committing--; | |||
|
|
|||
| if (max_actions > 0 && actions.size() > max_actions) { | |||
There was a problem hiding this comment.
Non-blocking:
Shouldn't this occur in create_action instead? Note that this is where _discard_redo() is used currently, and that it is where the action itself is created.
Also, note that the if does not need to check for actions.size() > max_actions as it is going to be checked by the while anyway.
| <return type="int"> | ||
| </return> | ||
| <description> | ||
| Gets the index of the current action. |
There was a problem hiding this comment.
Is there a link between the "current action" and the amount of undo steps available? If so, the current documentation does not mention it.
|
Also, note that there is no proposal backing this feature-adding PR. As there has not been discussion about it before, it is possible that it does not get merged with the current API (even after the changes I suggested). I would suggest opening one, for the sake of completeness. |
ok. Its more important that you get the idea. (i see the problems with it. It was for sure only a dirty hack to make it possible. It would need to make more coding around it, so it is not become inconstancy - on changing the max_actions value it has to delete elements from the stack as well, off course. And other stuff) |
|
This PR needs to be redone if there is any interest in it. UndoRedo has changed a lot since this was last worked on. |
int get_action_count() const;int get_current_action() const;void set_max_actions(int p_max_actions);int get_max_actions() const;