Fix order of cmds added when a new layer is created (fix #3736)

As creating a new layer was generating a onActiveSiteChange() event,
if a script was listening that event and adding new
actions (e.g. changing the layer properties), the order of cmds was
incorrectly created (e.g. cmd::SetUserDataProperty for the layer, and
then the cmd::AddLayer).

With this change we first add the cmd::AddLayer and then any extra
cmd::SetUserDataProperty (or any other action) that can be added in
the sitechange event.

We've created ev.fromUndo flag to the app 'sitechange' Lua event so
scripts can detect if we are undoing/redoing in a site change event,
and avoid adding new properties/actions when this happens.

And we have added a new exception (CannotModifyWhenUndoingException)
to detect if a new action/cmd is added (incorrectly) when we are
undoing/redoing.

Related to #3720

src/app/cmd_sequence.cpp

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2023  Igara Studio S.A.
 // Copyright (C) 2001-2015  David Capello
 //
 // This program is distributed under the terms of
@@ -27,6 +28,30 @@ void CmdSequence::add(Cmd* cmd)
   m_cmds.push_back(cmd);
 }
 
+void CmdSequence::addAndExecute(Context* ctx, Cmd* cmd)
+{
+  // First we add the cmd to the list of commands (m_cmds). In this way
+  add(cmd);
+
+  // Index where the cmd was added just in case to remove it if we
+  // catch an exception.
+  const int i = m_cmds.size()-1;
+
+  try {
+    // After we've added the cmd to the cmds list, we can execute
+    // it. As this execution can generate signals/notifications (like
+    // onActiveSiteChange), those who are listening to those
+    // notifications can add and execute more cmds (and we have to add
+    // all of them in order, that's why the cmd was added in m_cmds in
+    // the first place).
+    cmd->execute(ctx);
+  }
+  catch (...) {
+    m_cmds.erase(m_cmds.begin() + i);
+    throw;
+  }
+}
+
 void CmdSequence::onExecute()
 {
   for (auto it = m_cmds.begin(), end=m_cmds.end(); it!=end; ++it)
@@ -57,8 +82,7 @@ size_t CmdSequence::onMemSize() const
 
 void CmdSequence::executeAndAdd(Cmd* cmd)
 {
-  cmd->execute(context());
-  add(cmd);
+  addAndExecute(context(), cmd);
 }
 
 } // namespace app

src/app/cmd_sequence.h

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2023  Igara Studio S.A.
 // Copyright (C) 2001-2015  David Capello
 //
 // This program is distributed under the terms of
@@ -20,6 +21,7 @@ namespace app {
     ~CmdSequence();
 
     void add(Cmd* cmd);
+    void addAndExecute(Context* ctx, Cmd* cmd);
 
     // Helper to create a CmdSequence in the same onExecute() member
     // function.

src/app/doc.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2022  Igara Studio S.A.
+// Copyright (C) 2018-2023  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -156,6 +156,11 @@ DocApi Doc::getApi(Transaction& transaction)
 //////////////////////////////////////////////////////////////////////
 // Main properties
 
+bool Doc::isUndoing() const
+{
+  return m_undo->isUndoing();
+}
+
 color_t Doc::bgColor() const
 {
   return color_utils::color_for_target(

src/app/doc.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2020  Igara Studio S.A.
+// Copyright (C) 2018-2023  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -96,6 +96,8 @@ namespace app {
     const DocUndo* undoHistory() const { return m_undo.get(); }
     DocUndo* undoHistory() { return m_undo.get(); }
 
+    bool isUndoing() const;
+
     color_t bgColor() const;
     color_t bgColor(Layer* layer) const;
 

src/app/doc_undo.cpp

@@ -46,7 +46,8 @@ void DocUndo::add(CmdTransaction* cmd)
   ASSERT(cmd);
 
   if (m_undoing) {
-    Console(m_ctx).printf("Error running scripts: Adding undo information when navigating undo history");
+    delete cmd;
+    throw CannotModifyWhenUndoingException();
   }
 
   UNDO_TRACE("UNDO: Add state <%s> of %s to %s\n",

src/app/doc_undo.h

@@ -12,6 +12,7 @@
 #include "app/doc_range.h"
 #include "app/sprite_position.h"
 #include "base/disable_copying.h"
+#include "base/exception.h"
 #include "obs/observable.h"
 #include "undo/undo_history.h"
 
@@ -26,6 +27,15 @@ namespace app {
   class Context;
   class DocUndoObserver;
 
+  // Exception thrown when we want to modify the sprite (add new
+  // app::Cmd objects) when we are undoing/redoing/moving throw the
+  // undo history.
+  class CannotModifyWhenUndoingException : public base::Exception {
+  public:
+    CannotModifyWhenUndoingException() throw()
+    : base::Exception("Cannot modify the sprite when we are undoing/redoing an action.") { }
+  };
+
   class DocUndo : public obs::observable<DocUndoObserver>,
                   public undo::UndoHistoryDelegate {
   public:
@@ -86,6 +96,8 @@ namespace app {
     const undo::UndoState* lastState() const    { return m_undoHistory.lastState(); }
     const undo::UndoState* currentState() const { return m_undoHistory.currentState(); }
 
+    bool isUndoing() const { return m_undoing; }
+
     void moveToState(const undo::UndoState* state);
 
   private:

src/app/script/events_class.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2021-2022  Igara Studio S.A.
+// Copyright (C) 2021-2023  Igara Studio S.A.
 //
 // This program is distributed under the terms of
 // the End-User License Agreement for Aseprite.
@@ -20,6 +20,7 @@
 #include "app/script/engine.h"
 #include "app/script/luacpp.h"
 #include "app/script/values.h"
+#include "app/site.h"
 #include "doc/document.h"
 #include "doc/sprite.h"
 #include "ui/app_state.h"
@@ -204,7 +205,9 @@ private:
 
   // ContextObserver impl
   void onActiveSiteChange(const Site& site) override {
-    call(SiteChange);
+    const bool fromUndo = (site.document() &&
+                           site.document()->isUndoing());
+    call(SiteChange, { { "fromUndo", fromUndo } });
   }
 
   obs::scoped_connection m_fgConn;

src/app/transaction.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2022  Igara Studio S.A.
+// Copyright (C) 2018-2023  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -140,19 +140,23 @@ void Transaction::rollback(CmdTransaction* newCmds)
 
 void Transaction::execute(Cmd* cmd)
 {
-  try {
-    cmd->execute(m_ctx);
-  }
-  catch (...) {
+  // If we are undoing/redoing, just throw an exception, we cannot
+  // modify the sprite while we are moving throw the undo history.
+  // To undo/redo we have just to call the onUndo/onRedo of each
+  // app::Cmd.
+  if (m_doc->isUndoing()) {
     delete cmd;
-    throw;
+    throw CannotModifyWhenUndoingException();
   }
 
   try {
-    m_cmds->add(cmd);
+    // We have to add the "cmd" to the sequence (CmdTransaction) and
+    // then execute it. This is because the execution can generate
+    // some signals that could add/execute new actions to the undo
+    // history/sequence.
+    m_cmds->addAndExecute(m_ctx, cmd);
   }
   catch (...) {
-    cmd->undo();
     delete cmd;
     throw;
   }