Merge fe39e0cd86 into 2ba051b59b

Although the issue refers to deselecting MovingPixelsState, the same
crash could happen when canceling/finishing WritingTextState or
MovingSelectionState. This fixes the crash for all these states.

src/app/script/site_class.cpp

@@ -22,7 +22,6 @@
 #include "doc/cel.h"
 #include "doc/layer.h"
 #include "doc/sprite.h"
-#include "undo/undo_state.h"
 
 namespace app { namespace script {
 
@@ -105,45 +104,6 @@ int Site_get_tilesetMode(lua_State* L)
   return 1;
 }
 
-int Site_get_undoHistory(lua_State* L)
-{
-  const auto* site = get_obj<Site>(L, 1);
-  if (!site || !site->document()) {
-    lua_pushnil(L);
-    return 1;
-  }
-
-  const auto* history = site->document()->undoHistory();
-  if (!history) {
-    lua_pushnil(L);
-    return 1;
-  }
-
-  const undo::UndoState* currentState = history->currentState();
-  const undo::UndoState* s = history->firstState();
-  const bool canRedo = history->canRedo();
-  bool pastCurrent = !currentState && canRedo;
-
-  int undoStates = 0;
-  int redoStates = 0;
-  while (s) {
-    if (pastCurrent && canRedo)
-      redoStates++;
-    else if (currentState || !canRedo)
-      undoStates++;
-
-    if (s == currentState || !currentState)
-      pastCurrent = true;
-
-    s = s->next();
-  }
-
-  lua_newtable(L);
-  setfield_integer(L, "undoStates", undoStates);
-  setfield_integer(L, "redoStates", redoStates);
-  return 1;
-}
-
 const luaL_Reg Site_methods[] = {
   { nullptr, nullptr }
 };
@@ -157,7 +117,6 @@ const Property Site_properties[] = {
   { "image",       Site_get_image,       nullptr },
   { "tilemapMode", Site_get_tilemapMode, nullptr },
   { "tilesetMode", Site_get_tilesetMode, nullptr },
-  { "undoHistory", Site_get_undoHistory, nullptr },
   { nullptr,       nullptr,              nullptr }
 };
 

src/app/script/sprite_class.cpp

@@ -64,6 +64,7 @@
 #include "doc/tag.h"
 #include "doc/tileset.h"
 #include "doc/tilesets.h"
+#include "undo/undo_state.h"
 
 #include <algorithm>
 
@@ -1029,6 +1030,42 @@ int Sprite_set_useLayerUuids(lua_State* L)
   return 0;
 }
 
+int Sprite_get_undoHistory(lua_State* L)
+{
+  const auto* sprite = get_docobj<Sprite>(L, 1);
+  const auto* doc = static_cast<Doc*>(sprite->document());
+  const auto* history = doc->undoHistory();
+
+  if (!history) {
+    lua_pushnil(L);
+    return 1;
+  }
+
+  const undo::UndoState* currentState = history->currentState();
+  const undo::UndoState* s = history->firstState();
+  const bool canRedo = history->canRedo();
+  bool pastCurrent = !currentState && canRedo;
+
+  int undoSteps = 0;
+  int redoSteps = 0;
+  while (s) {
+    if (pastCurrent && canRedo)
+      redoSteps++;
+    else if (currentState || !canRedo)
+      undoSteps++;
+
+    if (s == currentState || !currentState)
+      pastCurrent = true;
+
+    s = s->next();
+  }
+
+  lua_newtable(L);
+  setfield_integer(L, "undoSteps", undoSteps);
+  setfield_integer(L, "redoSteps", redoSteps);
+  return 1;
+}
+
 const luaL_Reg Sprite_methods[] = {
   { "__eq",              Sprite_eq                },
   { "resize",            Sprite_resize            },
@@ -1094,6 +1131,7 @@ const Property Sprite_properties[] = {
   { "events",               Sprite_get_events,               nullptr                         },
   { "tileManagementPlugin", Sprite_get_tileManagementPlugin, Sprite_set_tileManagementPlugin },
   { "useLayerUuids",        Sprite_get_useLayerUuids,        Sprite_set_useLayerUuids        },
+  { "undoHistory",          Sprite_get_undoHistory,          nullptr                         },
   { nullptr,                nullptr,                         nullptr                         }
 };
 

src/app/ui/editor/editor.cpp

@@ -58,6 +58,7 @@
 #include "app/util/tile_flags_utils.h"
 #include "base/chrono.h"
 #include "base/convert_to.h"
+#include "base/scoped_value.h"
 #include "doc/doc.h"
 #include "doc/mask_boundaries.h"
 #include "doc/slice.h"
@@ -266,6 +267,23 @@ void Editor::setStateInternal(const EditorStatePtr& newState)
 {
   m_brushPreview.hide();
 
+  // Some onLeaveState impls (like the ones from MovingPixelsState,
+  // WritingTextState, MovingSelectionState) might generate a
+  // Tx/Transaction::commit(), which will add a new undo state,
+  // triggering a sprite change scripting event
+  // (SpriteEvents::onAddUndoState). This event could be handled by an
+  // extension and that extension might want to save the current
+  // sprite (e.g. calling Sprite_saveCopyAs, the kind of extension
+  // that takes snapshots after each sprite change). That will be a
+  // new Context::executeCommand() for the save command, generating a
+  // BeforeCommandExecution signal, getting back to onLeaveState
+  // again. In that case, we just ignore the reentry as the first
+  // onLeaveState should handle everything (to avoid an stack
+  // overflow/infinite recursion).
+  if (m_leavingState)
+    return;
+  base::ScopedValue leaving(m_leavingState, true);
+
   // Fire before change state event, set the state, and fire after
   // change state event.
   EditorState::LeaveAction leaveAction = m_state->onLeaveState(this, newState.get());

src/app/ui/editor/editor.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2024  Igara Studio S.A.
+// Copyright (C) 2018-2025  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -458,6 +458,13 @@ private:
 
   DocView* m_docView;
 
+  // Special flag to avoid re-entering a new state when we are leaving
+  // the current one. This avoids an infinite onLeaveState() recursion
+  // in some special cases when an extension (third-party code)
+  // creates a new sprite change in the same sprite change scripting
+  // event.
+  bool m_leavingState = false;
+
   // Last known mouse position received by this editor when the
   // mouse button was pressed. Used for auto-scrolling. To get the
   // current mouse position on the editor you can use

tests/scripts/site.lua

@@ -1,54 +0,0 @@
--- Copyright (C) 2019-2025  Igara Studio S.A.
---
--- This file is released under the terms of the MIT license.
--- Read LICENSE.txt for more information.
-
-do
-  assert(not app.site.sprite)
-  assert(app.site.undoHistory == nil)
-
-  local a = Sprite(1, 1)
-
-  assert(a == app.site.sprite)
-
-  assert(app.site.undoHistory.undoStates == 0)
-  assert(app.site.undoHistory.redoStates == 0)
-
-  a:resize(10, 10)
-
-  assert(app.site.undoHistory.undoStates == 1)
-  assert(app.site.undoHistory.redoStates == 0)
-
-  a:resize(10, 15)
-
-  assert(app.site.undoHistory.undoStates == 2)
-  assert(app.site.undoHistory.redoStates == 0)
-
-  a:resize(10, 30)
-
-  assert(app.site.undoHistory.undoStates == 3)
-  assert(app.site.undoHistory.redoStates == 0)
-
-  app.undo()
-  assert(app.site.undoHistory.undoStates == 2)
-  assert(app.site.undoHistory.redoStates == 1)
-
-  app.undo()
-  assert(app.site.undoHistory.undoStates == 1)
-  assert(app.site.undoHistory.redoStates == 2)
-
-  app.redo()
-  assert(app.site.undoHistory.undoStates == 2)
-  assert(app.site.undoHistory.redoStates == 1)
-
-  app.undo()
-  app.undo()
-
-  assert(app.site.undoHistory.undoStates == 0)
-  assert(app.site.undoHistory.redoStates == 3)
-
-  a:resize(10, 30)
-
-  assert(app.site.undoHistory.undoStates == 1)
-  assert(app.site.undoHistory.redoStates == 0)
-end

tests/scripts/sprite.lua

@@ -228,3 +228,49 @@ do
   c = app.open(fn)
   assert(c.tileManagementPlugin == nil)
 end
+
+-- Undo History
+do
+  local sprite = Sprite(1, 1)
+
+  assert(sprite.undoHistory.undoSteps == 0)
+  assert(sprite.undoHistory.redoSteps == 0)
+
+  sprite:resize(10, 10)
+
+  assert(sprite.undoHistory.undoSteps == 1)
+  assert(sprite.undoHistory.redoSteps == 0)
+
+  sprite:resize(10, 15)
+
+  assert(sprite.undoHistory.undoSteps == 2)
+  assert(sprite.undoHistory.redoSteps == 0)
+
+  sprite:resize(10, 30)
+
+  assert(sprite.undoHistory.undoSteps == 3)
+  assert(sprite.undoHistory.redoSteps == 0)
+
+  app.undo()
+  assert(sprite.undoHistory.undoSteps == 2)
+  assert(sprite.undoHistory.redoSteps == 1)
+
+  app.undo()
+  assert(sprite.undoHistory.undoSteps == 1)
+  assert(sprite.undoHistory.redoSteps == 2)
+
+  app.redo()
+  assert(sprite.undoHistory.undoSteps == 2)
+  assert(sprite.undoHistory.redoSteps == 1)
+
+  app.undo()
+  app.undo()
+
+  assert(sprite.undoHistory.undoSteps == 0)
+  assert(sprite.undoHistory.redoSteps == 3)
+
+  sprite:resize(10, 30)
+
+  assert(sprite.undoHistory.undoSteps == 1)
+  assert(sprite.undoHistory.redoSteps == 0)
+end