Skip to content

[lua] Add sprite.undoHistory#5294

Merged
dacap merged 1 commit into
aseprite:mainfrom
ckaiser:app-site-undohistory
Jul 25, 2025
Merged

[lua] Add sprite.undoHistory#5294
dacap merged 1 commit into
aseprite:mainfrom
ckaiser:app-site-undohistory

Conversation

@ckaiser

@ckaiser ckaiser commented Jul 23, 2025

Copy link
Copy Markdown
Member

Implements #5205. I'm not 100% convinced by the API names, alternatively we can use something like undoStatesCount or call them undoSteps (or stepCount) -- I also figured Site was the place to put this but it could be moved without much issue as long as we have document access, it makes sense to me since it's a level above the sprite and scripting has no concept of a "document".

@ckaiser ckaiser requested a review from dacap as a code owner July 23, 2025 13:18

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/app/script/site_class.cpp Outdated
@dacap

dacap commented Jul 23, 2025

Copy link
Copy Markdown
Member

Implements #5205, I'm not 100% convinced by the API names, alternatively we can use something like undoStatesCount or call them undoSteps (or stepCount)

I think undoHistory.undoSteps and undoHistory.redoSteps might be a good option.

In a future we could have undoHistory.undoStates (or undoHistory.states) for a future API to get/move to a specific position/state in the undo history. But that requires several new objects to make it useful.

I also figured Site was the place to put this but it could be moved without much issue as long as we have document access, it makes sense to me since it's a level above the sprite and scripting has no concept of a "document".

Site is like a snapshot of the editor/timeline position in a specific time (app.site the current site), it's not updated when it's stored. Actually the best position for the undoHistory is the Sprite. So we can do sprite.undoHistory.

In some way the internal app::Doc + doc::Sprite is represented by the scripting Sprite (check sprite_class.cpp functions to see how the document is obtained from the sprite).

@dacap dacap assigned ckaiser and unassigned dacap Jul 23, 2025
@ckaiser ckaiser force-pushed the app-site-undohistory branch from a30a4cd to fe39e0c Compare July 23, 2025 23:15

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/app/script/sprite_class.cpp
@ckaiser ckaiser changed the title [lua] Add site.undoHistory [lua] Add sprite.undoHistory Jul 23, 2025
@ckaiser

ckaiser commented Jul 23, 2025

Copy link
Copy Markdown
Member Author

Renamed and moved to Sprite

@ckaiser ckaiser assigned dacap and unassigned ckaiser Jul 23, 2025
Comment thread tests/scripts/sprite.lua Outdated
@dacap dacap assigned ckaiser and unassigned dacap Jul 25, 2025
@ckaiser ckaiser force-pushed the app-site-undohistory branch from fe39e0c to 7578fbd Compare July 25, 2025 15:22
@ckaiser ckaiser assigned dacap and unassigned ckaiser Jul 25, 2025
@dacap

dacap commented Jul 25, 2025

Copy link
Copy Markdown
Member

Looks great! 👍

@dacap dacap merged commit 80fa065 into aseprite:main Jul 25, 2025
12 checks passed
@ckaiser ckaiser deleted the app-site-undohistory branch September 10, 2025 05:18
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.

3 participants