Add 'beforesitechange' event (fix #4569)

src/app/context.cpp

@@ -137,9 +137,15 @@ bool Context::hasModifiedDocuments() const
   return false;
 }
 
+void Context::notifyBeforeActiveSiteChanged()
+{
+  const Site site = activeSite();
+  notify_observers<const Site&>(&ContextObserver::onBeforeActiveSiteChange, site);
+}
+
 void Context::notifyActiveSiteChanged()
 {
-  Site site = activeSite();
+  const Site site = activeSite();
   notify_observers<const Site&>(&ContextObserver::onActiveSiteChange, site);
 }
 
@@ -250,6 +256,11 @@ void Context::setCommandResult(const CommandResult& result)
   m_result = result;
 }
 
+void Context::onBeforeAddDocument(Doc* doc)
+{
+  notifyBeforeActiveSiteChanged();
+}
+
 void Context::onAddDocument(Doc* doc)
 {
   m_lastSelectedDoc = doc;
@@ -260,6 +271,11 @@ void Context::onAddDocument(Doc* doc)
   notifyActiveSiteChanged();
 }
 
+void Context::onBeforeRemoveDocument(Doc* doc)
+{
+  notifyBeforeActiveSiteChanged();
+}
+
 void Context::onRemoveDocument(Doc* doc)
 {
   if (m_activeSiteHandler)

src/app/context.h

@@ -140,6 +140,7 @@ namespace app {
     void setSelectedTiles(const doc::PalettePicks& picks);
     bool hasModifiedDocuments() const;
     void notifyActiveSiteChanged();
+    void notifyBeforeActiveSiteChanged();
 
     void setDraggedData(std::unique_ptr<DraggedData> draggedData) {
       m_draggedData = std::move(draggedData);
@@ -161,7 +162,9 @@ namespace app {
 
   protected:
     // DocsObserver impl
+    void onBeforeAddDocument(Doc* doc) override;
     void onAddDocument(Doc* doc) override;
+    void onBeforeRemoveDocument(Doc* doc) override;
     void onRemoveDocument(Doc* doc) override;
 
     virtual void onGetActiveSite(Site* site) const;

src/app/context_observer.h

@@ -16,7 +16,8 @@ namespace app {
   class ContextObserver {
   public:
     virtual ~ContextObserver() { }
-    virtual void onActiveSiteChange(const Site& site) { }
+    virtual void onActiveSiteChange(const Site& site) { };
+    virtual void onBeforeActiveSiteChange(const Site& fromSite) { };
   };
 
 } // namespace app

src/app/docs.cpp

@@ -40,6 +40,8 @@ Doc* Docs::add(Doc* doc)
     return doc;
   }
 
+  notify_observers(&DocsObserver::onBeforeAddDocument, doc);
+
   m_docs.insert(begin(), doc);
 
   notify_observers(&DocsObserver::onAddDocument, doc);
@@ -62,6 +64,8 @@ void Docs::remove(Doc* doc)
   if (it == end())              // Already removed.
     return;
 
+  notify_observers(&DocsObserver::onBeforeRemoveDocument, doc);
+
   m_docs.erase(it);
 
   notify_observers(&DocsObserver::onRemoveDocument, doc);

src/app/docs_observer.h

@@ -24,7 +24,9 @@ namespace app {
   class DocsObserver {
   public:
     virtual ~DocsObserver() { }
+    virtual void onBeforeAddDocument(Doc* doc) { }
     virtual void onAddDocument(Doc* doc) { }
+    virtual void onBeforeRemoveDocument(Doc* doc) { }
     virtual void onRemoveDocument(Doc* doc) { }
   };
 

src/app/script/events_class.cpp

@@ -157,6 +157,7 @@ class AppEvents : public Events
 public:
   enum : EventType {
     Unknown = -1,
+    BeforeSiteChange,
     SiteChange,
     FgColorChange,
     BgColorChange,
@@ -164,12 +165,16 @@ public:
     AfterCommand,
   };
 
-  AppEvents() {
+  AppEvents()
+    : m_addedObserver(0)
+  {
   }
 
   EventType eventType(const char* eventName) const override {
     if (std::strcmp(eventName, "sitechange") == 0)
       return SiteChange;
+    else if (std::strcmp(eventName, "beforesitechange") == 0)
+      return BeforeSiteChange;
     else if (std::strcmp(eventName, "fgcolorchange") == 0)
       return FgColorChange;
     else if (std::strcmp(eventName, "bgcolorchange") == 0)
@@ -189,9 +194,14 @@ private:
     auto ctx = app->context();
     auto& pref = Preferences::instance();
     switch (eventType) {
-      case SiteChange:
-        ctx->add_observer(this);
-        break;
+      case BeforeSiteChange:
+        [[fallthrough]];
+      case SiteChange: {
+        if (m_addedObserver == 0)
+          ctx->add_observer(this);
+
+        ++m_addedObserver;
+      }  break;
       case FgColorChange:
         m_fgConn = pref.colorBar.fgColor.AfterChange
           .connect([this]{ onFgColorChange(); });
@@ -213,8 +223,13 @@ private:
 
   void onRemoveLastListener(EventType eventType) override {
     switch (eventType) {
+      case BeforeSiteChange:
+        [[fallthrough]];
       case SiteChange:
-        App::instance()->context()->remove_observer(this);
+        --m_addedObserver;
+
+        if (m_addedObserver == 0)
+          App::instance()->context()->remove_observer(this);
         break;
       case FgColorChange:
         m_fgConn.disconnect();
@@ -265,11 +280,19 @@ private:
     call(SiteChange, { { "fromUndo", fromUndo } });
   }
 
+  void onBeforeActiveSiteChange(const Site& fromSite) override
+  {
+    const bool fromUndo = (fromSite.document() &&
+                           fromSite.document()->isUndoing());
+    call(BeforeSiteChange, { { "fromUndo", fromUndo } });
+  }
+
   obs::scoped_connection m_fgConn;
   obs::scoped_connection m_bgConn;
   obs::scoped_connection m_beforeCmdConn;
   obs::scoped_connection m_afterCmdConn;
   obs::scoped_connection m_beforePaintConn;
+  int m_addedObserver;
 };
 
 class WindowEvents : public Events

src/app/script/values.cpp

@@ -8,8 +8,8 @@
 #include "config.h"
 #endif
 
+#include "app/site.h"
 #include "app/script/values.h"
-
 #include "app/pref/preferences.h"
 #include "app/script/docobj.h"
 #include "app/script/engine.h"

src/app/ui/editor/editor.cpp

@@ -361,6 +361,9 @@ void Editor::setLayer(const Layer* layer)
   if (gridVisible)
     oldGrid = getSite().grid();
 
+  if (isActive())
+    UIContext::instance()->notifyBeforeActiveSiteChanged();
+
   m_observers.notifyBeforeLayerChanged(this);
 
   // Remove extra cel information if we change between different layer
@@ -409,6 +412,9 @@ void Editor::setFrame(frame_t frame)
   if (m_frame == frame)
     return;
 
+  if (isActive())
+    UIContext::instance()->notifyBeforeActiveSiteChanged();
+
   m_observers.notifyBeforeFrameChanged(this);
   {
     HideBrushPreview hide(m_brushPreview);
@@ -1914,6 +1920,9 @@ bool Editor::isSliceSelected(const doc::Slice* slice) const
 void Editor::clearSlicesSelection()
 {
   if (!m_selectedSlices.empty()) {
+    if (isActive())
+      UIContext::instance()->notifyBeforeActiveSiteChanged();
+
     m_selectedSlices.clear();
     invalidate();
 
@@ -1925,6 +1934,9 @@ void Editor::clearSlicesSelection()
 void Editor::selectSlice(const doc::Slice* slice)
 {
   ASSERT(slice);
+  if (isActive())
+    UIContext::instance()->notifyBeforeActiveSiteChanged();
+
   m_selectedSlices.insert(slice->id());
   invalidate();
 
@@ -1934,6 +1946,9 @@ void Editor::selectSlice(const doc::Slice* slice)
 
 bool Editor::selectSliceBox(const gfx::Rect& box)
 {
+  if (isActive())
+    UIContext::instance()->notifyBeforeActiveSiteChanged();
+
   m_selectedSlices.clear();
   for (auto slice : m_sprite->slices()) {
     auto key = slice->getByFrame(m_frame);
@@ -1950,6 +1965,9 @@ bool Editor::selectSliceBox(const gfx::Rect& box)
 
 void Editor::selectAllSlices()
 {
+  if (isActive())
+    UIContext::instance()->notifyBeforeActiveSiteChanged();
+
   for (auto slice : m_sprite->slices())
     m_selectedSlices.insert(slice->id());
   invalidate();

src/app/ui/main_window.cpp

@@ -134,6 +134,7 @@ void MainWindow::initialize()
   m_timeline = new Timeline(m_tooltipManager);
 
   m_workspace->setTabsBar(m_tabsBar);
+  m_workspace->BeforeViewChanged.connect(&MainWindow::onBeforeViewChange, this);
   m_workspace->ActiveViewChanged.connect(&MainWindow::onActiveViewChange, this);
 
   // configure all widgets to expansives
@@ -417,6 +418,11 @@ void MainWindow::onResize(ui::ResizeEvent& ev)
   }
 }
 
+void MainWindow::onBeforeViewChange()
+{
+  UIContext::instance()->notifyBeforeActiveSiteChanged();
+}
+
 // When the active view is changed from methods like
 // Workspace::splitView(), this function is called, and we have to
 // inform to the UIContext that the current view has changed.

src/app/ui/main_window.h

@@ -116,6 +116,7 @@ namespace app {
     void onInitTheme(ui::InitThemeEvent& ev) override;
     void onSaveLayout(ui::SaveLayoutEvent& ev) override;
     void onResize(ui::ResizeEvent& ev) override;
+    void onBeforeViewChange();
     void onActiveViewChange();
     void onLanguageChange();
 

src/app/ui/workspace.cpp

@@ -100,6 +100,8 @@ void Workspace::setActiveView(WorkspaceView* view)
   if (!m_activePanel)
     return;
 
+  BeforeViewChanged();
+
   m_activePanel->setActiveView(view);
 
   ActiveViewChanged();          // Fire ActiveViewChanged event
@@ -107,6 +109,8 @@ void Workspace::setActiveView(WorkspaceView* view)
 
 void Workspace::setMainPanelAsActive()
 {
+  BeforeViewChanged();
+
   m_activePanel = &m_mainPanel;
 
   removeDropViewPreview();

src/app/ui/workspace.h

@@ -78,6 +78,7 @@ namespace app {
 
     WorkspacePanel* mainPanel() { return &m_mainPanel; }
 
+    obs::signal<void()> BeforeViewChanged;
     obs::signal<void()> ActiveViewChanged;
 
   protected:

src/app/ui_context.cpp

@@ -144,6 +144,10 @@ void UIContext::setActiveView(DocView* docView)
 void UIContext::onSetActiveDocument(Doc* document, bool notify)
 {
   notify = (notify && lastSelectedDoc() != document);
+
+  if (notify)
+    notifyBeforeActiveSiteChanged();
+
   app::Context::onSetActiveDocument(document, false);
 
   DocView* docView = getFirstDocView(document);

tests/scripts/events.lua

@@ -7,21 +7,69 @@ dofile('./test_utils.lua')
 
 -- Test app.events
 do
-  local i = 0
+  local bc = 0
+  local c = 0
+  local beforeListener = app.events:on('beforesitechange',
+                                 function() bc = bc + 1 end)
   local listener = app.events:on('sitechange',
-                                 function() i = i + 1 end)
-  assert(i == 0)
+                                 function() c = c + 1 end)
+
+  assert(bc == 0)
+  assert(c == 0)
   local a = Sprite(32, 32)
   expect_eq(a, app.activeSprite)
-  expect_eq(1, i)
+  expect_eq(1, bc)
+  expect_eq(1, c)
+
   local b = Sprite(32, 32)
   expect_eq(b, app.activeSprite)
-  expect_eq(2, i)
+  expect_eq(2, bc)
+  expect_eq(2, c)
+
   app.activeSprite = a
-  expect_eq(3, i)
+  expect_eq(3, bc)
+  expect_eq(3, c)
+
   app.events:off(listener)
+  app.events:off(beforeListener)
+
   app.activeSprite = b
-  expect_eq(3, i)
+  expect_eq(3, bc)
+  expect_eq(3, c)
+end
+
+-- Alternate version of the events test to ensure proper observer disconnection
+do
+  local bc = 0
+  local c = 0
+  local beforeListener = app.events:on('beforesitechange',
+                                 function() bc = bc + 1 end)
+  local listener = app.events:on('sitechange',
+                                 function() c = c + 1 end)
+
+  assert(bc == 0)
+  assert(c == 0)
+  local a = Sprite(32, 32)
+  expect_eq(a, app.activeSprite)
+  expect_eq(1, bc)
+  expect_eq(1, c)
+
+  app.events:off(beforeListener)
+
+  local b = Sprite(32, 32)
+  expect_eq(b, app.activeSprite)
+  expect_eq(1, bc)
+  expect_eq(2, c)
+
+  app.activeSprite = a
+  expect_eq(1, bc)
+  expect_eq(3, c)
+
+  app.events:off(listener)
+
+  app.activeSprite = b
+  expect_eq(1, bc)
+  expect_eq(3, c)
 end
 
 do