Enable commands when we're in MovingPixelsState

Now ContextFlags enable some flags if it detects that the current editor
is in MovingPixelsState. In this way commands "think" that they are able
to lock the document for write access (finally they will be able, because
MovingPixelsState::onBeforeCommandExecution() unlocks the document
just before the command is executed).

Fix #275, fix #690, fix #750

src/app/context.cpp

@@ -53,39 +53,47 @@ void Context::executeCommand(Command* command, const Params& params)
 
   ASSERT(command != NULL);
 
-  PRINTF("Executing '%s' command.\n", command->id().c_str());
+  PRINTF("Context: Executing command '%s'...\n", command->id().c_str());
-  BeforeCommandExecution(command);
-
   try {
     m_flags.update(this);
 
     command->loadParams(params);
 
-    if (command->isEnabled(this)) {
+    CommandExecutionEvent ev(command);
-      command->execute(this);
+    BeforeCommandExecution(ev);
 
-      AfterCommandExecution(command);
+    if (ev.isCanceled()) {
+      PRINTF("Context: '%s' was canceled/simulated.\n", command->id().c_str());
     }
+    else if (command->isEnabled(this)) {
+      command->execute(this);
+      PRINTF("Context: '%s' executed successfully\n", command->id().c_str());
+    }
+    else {
+      PRINTF("Context: '%s' is disabled\n", command->id().c_str());
+    }
+
+    AfterCommandExecution(ev);
 
     // TODO move this code to another place (e.g. a Workplace/Tabs widget)
     if (isUIAvailable())
       app_rebuild_documents_tabs();
   }
   catch (base::Exception& e) {
-    PRINTF("Exception caught executing '%s' command\n%s\n",
+    PRINTF("Context: Exception caught executing '%s' command\n%s\n",
            command->id().c_str(), e.what());
 
     Console::showException(e);
   }
   catch (std::exception& e) {
-    PRINTF("std::exception caught executing '%s' command\n%s\n",
+    PRINTF("Context: std::exception caught executing '%s' command\n%s\n",
            command->id().c_str(), e.what());
 
     console.printf("An error ocurred executing the command.\n\nDetails:\n%s", e.what());
   }
 #ifndef DEBUGMODE
   catch (...) {
-    PRINTF("unknown exception executing '%s' command\n",
+    PRINTF("Context: Unknown exception executing '%s' command\n",
            command->id().c_str());
 
     console.printf("An unknown error ocurred executing the command.\n"

src/app/context.h

@@ -28,6 +28,26 @@ namespace app {
     : base::Exception("Cannot execute the command because its pre-conditions are false.") { }
   };
 
+  class CommandExecutionEvent {
+  public:
+    CommandExecutionEvent(Command* command)
+      : m_command(command), m_canceled(false) {
+    }
+
+    Command* command() const { return m_command; }
+
+    // True if the command was canceled or simulated by an
+    // observer/signal slot.
+    bool isCanceled() const { return m_canceled; }
+    void cancel() {
+      m_canceled = true;
+    }
+
+  private:
+    Command* m_command;
+    bool m_canceled;
+  };
+
   class Context : public doc::Context {
   public:
     Context();
@@ -47,8 +67,8 @@ namespace app {
     void executeCommand(const char* commandName);
     virtual void executeCommand(Command* command, const Params& params = Params());
 
-    Signal1<void, Command*> BeforeCommandExecution;
+    Signal1<void, CommandExecutionEvent&> BeforeCommandExecution;
-    Signal1<void, Command*> AfterCommandExecution;
+    Signal1<void, CommandExecutionEvent&> AfterCommandExecution;
 
   protected:
     virtual void onCreateDocument(doc::CreateDocumentArgs* args) override;

src/app/context_flags.cpp

@@ -13,6 +13,8 @@
 
 #include "app/context.h"
 #include "app/document.h"
+#include "app/modules/editors.h"
+#include "app/ui/editor/editor.h"
 #include "doc/cel.h"
 #include "doc/layer.h"
 #include "doc/site.h"
@@ -41,45 +43,67 @@ void ContextFlags::update(Context* context)
       if (document->isMaskVisible())
         m_flags |= HasVisibleMask;
 
-      Sprite* sprite = site.sprite();
+      updateFlagsFromSite(site);
-      if (sprite) {
-        m_flags |= HasActiveSprite;
-
-        if (sprite->backgroundLayer())
-          m_flags |= HasBackgroundLayer;
-
-        Layer* layer = site.layer();
-        if (layer) {
-          m_flags |= HasActiveLayer;
-
-          if (layer->isBackground())
-            m_flags |= ActiveLayerIsBackground;
-
-          if (layer->isVisible())
-            m_flags |= ActiveLayerIsVisible;
-
-          if (layer->isEditable())
-            m_flags |= ActiveLayerIsEditable;
-
-          if (layer->isImage()) {
-            m_flags |= ActiveLayerIsImage;
-
-            Cel* cel = layer->cel(site.frame());
-            if (cel) {
-              m_flags |= HasActiveCel;
-
-              if (cel->image())
-                m_flags |= HasActiveImage;
-            }
-          }
-        }
-      }
 
       if (document->lockToWrite(0))
         m_flags |= ActiveDocumentIsWritable;
 
       document->unlock();
     }
+
+    // TODO this is a hack, try to find a better design to handle this
+    // "moving pixels" state.
+    if (current_editor &&
+        current_editor->document() == document &&
+        current_editor->isMovingPixels()) {
+      // Flags enabled when we are in MovingPixelsState
+      m_flags |=
+        HasVisibleMask |
+        ActiveDocumentIsReadable |
+        ActiveDocumentIsWritable;
+
+      updateFlagsFromSite(current_editor->getSite());
+    }
+  }
+}
+
+void ContextFlags::updateFlagsFromSite(const Site& site)
+{
+  const Sprite* sprite = site.sprite();
+  if (!sprite)
+    return;
+
+  m_flags |= HasActiveSprite;
+
+  if (sprite->backgroundLayer())
+    m_flags |= HasBackgroundLayer;
+
+  const Layer* layer = site.layer();
+  frame_t frame = site.frame();
+  if (!layer)
+    return;
+
+  m_flags |= HasActiveLayer;
+
+  if (layer->isBackground())
+    m_flags |= ActiveLayerIsBackground;
+
+  if (layer->isVisible())
+    m_flags |= ActiveLayerIsVisible;
+
+  if (layer->isEditable())
+    m_flags |= ActiveLayerIsEditable;
+
+  if (layer->isImage()) {
+    m_flags |= ActiveLayerIsImage;
+
+    Cel* cel = layer->cel(frame);
+    if (cel) {
+      m_flags |= HasActiveCel;
+
+      if (cel->image())
+        m_flags |= HasActiveImage;
+    }
   }
 }
 

src/app/context_flags.h

@@ -9,6 +9,10 @@
 #define APP_CONTEXT_FLAGS_H_INCLUDED
 #pragma once
 
+namespace doc {
+  class Site;
+}
+
 namespace app {
 
   class Context;
@@ -37,6 +41,8 @@ namespace app {
     void update(Context* context);
 
   private:
+    void updateFlagsFromSite(const doc::Site& site);
+
     uint32_t m_flags;
   };
 

src/app/ui/color_bar.cpp

@@ -366,17 +366,17 @@ void ColorBar::onFocusPaletteView()
   App::instance()->inputChain().prioritize(this);
 }
 
-void ColorBar::onBeforeExecuteCommand(Command* command)
+void ColorBar::onBeforeExecuteCommand(CommandExecutionEvent& ev)
 {
-  if (command->id() == CommandId::SetPalette ||
+  if (ev.command()->id() == CommandId::SetPalette ||
-      command->id() == CommandId::LoadPalette)
+      ev.command()->id() == CommandId::LoadPalette)
     showRemap();
 }
 
-void ColorBar::onAfterExecuteCommand(Command* command)
+void ColorBar::onAfterExecuteCommand(CommandExecutionEvent& ev)
 {
-  if (command->id() == CommandId::Undo ||
+  if (ev.command()->id() == CommandId::Undo ||
-      command->id() == CommandId::Redo)
+      ev.command()->id() == CommandId::Redo)
     invalidate();
 }
 

src/app/ui/color_bar.h

@@ -31,7 +31,7 @@ namespace app {
   class ColorButton;
   class ColorSpectrum;
   class ColorWheel;
-  class Command;
+  class CommandExecutionEvent;
   class PaletteIndexChangeEvent;
   class PalettePopup;
   class PalettesLoader;
@@ -87,8 +87,8 @@ namespace app {
   protected:
     void onAppPaletteChange();
     void onFocusPaletteView();
-    void onBeforeExecuteCommand(Command* command);
+    void onBeforeExecuteCommand(CommandExecutionEvent& ev);
-    void onAfterExecuteCommand(Command* command);
+    void onAfterExecuteCommand(CommandExecutionEvent& ev);
     void onPaletteButtonClick();
     void onRemapButtonClick();
     void onPaletteIndexChange(PaletteIndexChangeEvent& ev);

src/app/ui/document_view.cpp

@@ -266,6 +266,9 @@ void DocumentView::onClonedFrom(WorkspaceView* from)
 
 bool DocumentView::onCloseView(Workspace* workspace)
 {
+  if (m_editor->isMovingPixels())
+    m_editor->cancelMovingPixels();
+
   // If there is another view for this document, just close the view.
   for (auto view : *workspace) {
     DocumentView* docView = dynamic_cast<DocumentView*>(view);
@@ -456,20 +459,21 @@ void DocumentView::onNewInputPriority(InputChainElement* element)
 
 bool DocumentView::onCanCut(Context* ctx)
 {
-  return ctx->checkFlags(ContextFlags::ActiveDocumentIsWritable |
+  return onCanCopy(ctx);
-                         ContextFlags::ActiveLayerIsVisible |
-                         ContextFlags::ActiveLayerIsEditable |
-                         ContextFlags::HasVisibleMask |
-                         ContextFlags::HasActiveImage);
 }
 
 bool DocumentView::onCanCopy(Context* ctx)
 {
-  return ctx->checkFlags(ContextFlags::ActiveDocumentIsWritable |
+  if (ctx->checkFlags(ContextFlags::ActiveDocumentIsWritable |
-                         ContextFlags::ActiveLayerIsVisible |
+                      ContextFlags::ActiveLayerIsVisible |
-                         ContextFlags::ActiveLayerIsEditable |
+                      ContextFlags::ActiveLayerIsEditable |
-                         ContextFlags::HasVisibleMask |
+                      ContextFlags::HasVisibleMask |
-                         ContextFlags::HasActiveImage);
+                      ContextFlags::HasActiveImage))
+    return true;
+  else if (m_editor->isMovingPixels())
+    return true;
+  else
+    return false;
 }
 
 bool DocumentView::onCanPaste(Context* ctx)
@@ -484,10 +488,17 @@ bool DocumentView::onCanPaste(Context* ctx)
 
 bool DocumentView::onCanClear(Context* ctx)
 {
-  return ctx->checkFlags(ContextFlags::ActiveDocumentIsWritable |
+  if (ctx->checkFlags(ContextFlags::ActiveDocumentIsWritable |
-                         ContextFlags::ActiveLayerIsVisible |
+                      ContextFlags::ActiveLayerIsVisible |
-                         ContextFlags::ActiveLayerIsEditable |
+                      ContextFlags::ActiveLayerIsEditable |
-                         ContextFlags::ActiveLayerIsImage);
+                      ContextFlags::ActiveLayerIsImage)) {
+    return true;
+  }
+  else if (m_editor->isMovingPixels()) {
+    return true;
+  }
+  else
+    return false;
 }
 
 bool DocumentView::onCut(Context* ctx)

src/app/ui/editor/editor.cpp

@@ -1596,4 +1596,15 @@ gfx::Point Editor::calcExtraPadding(const Zoom& zoom)
     return gfx::Point(0, 0);
 }
 
+bool Editor::isMovingPixels() const
+{
+  return (dynamic_cast<MovingPixelsState*>(m_state.get()) != nullptr);
+}
+
+void Editor::cancelMovingPixels()
+{
+  ASSERT(isMovingPixels());
+  backToPreviousState();
+}
+
 } // namespace app

src/app/ui/editor/editor.h

@@ -90,6 +90,9 @@ namespace app {
     // Returns the current state.
     EditorStatePtr getState() { return m_state; }
 
+    bool isMovingPixels() const;
+    void cancelMovingPixels();
+
     // Changes the state of the editor.
     void setState(const EditorStatePtr& newState);
 

src/app/ui/editor/moving_pixels_state.cpp

@@ -139,6 +139,8 @@ void MovingPixelsState::onEnterState(Editor* editor)
 
 EditorState::LeaveAction MovingPixelsState::onLeaveState(Editor* editor, EditorState* newState)
 {
+  PRINTF("MovingPixels: leave state\n");
+
   ASSERT(m_pixelsMovement);
   ASSERT(editor == m_editor);
 
@@ -371,56 +373,6 @@ bool MovingPixelsState::onKeyDown(Editor* editor, KeyMessage* msg)
 
     return true;
   }
-  else {
-    Command* command = NULL;
-    Params params;
-    if (KeyboardShortcuts::instance()
-          ->getCommandFromKeyMessage(msg, &command, &params)) {
-      // We accept zoom commands.
-      if (command->id() == CommandId::Zoom) {
-        UIContext::instance()->executeCommand(command, params);
-        return true;
-      }
-      // Intercept the "Cut" or "Copy" command to handle them locally
-      // with the current m_pixelsMovement data.
-      else if (command->id() == CommandId::Cut ||
-               command->id() == CommandId::Copy) {
-        // Copy the floating image to the clipboard.
-        {
-          Document* document = editor->document();
-          base::UniquePtr<Image> floatingImage;
-          base::UniquePtr<Mask> floatingMask;
-          m_pixelsMovement->getDraggedImageCopy(floatingImage, floatingMask);
-
-          clipboard::copy_image(floatingImage.get(),
-                                floatingMask.get(),
-                                document->sprite()->palette(editor->frame()));
-        }
-
-        // In case of "Cut" command.
-        if (command->id() == CommandId::Cut) {
-          // Discard the dragged image.
-          m_pixelsMovement->discardImage();
-          m_discarded = true;
-
-          // Quit from MovingPixelsState, back to standby.
-          editor->backToPreviousState();
-        }
-
-        // Return true because we've used the keyboard shortcut.
-        return true;
-      }
-      // Flip Horizontally/Vertically commands are handled manually to
-      // avoid dropping the floating region of pixels.
-      else if (command->id() == CommandId::Flip) {
-        if (FlipCommand* flipCommand = dynamic_cast<FlipCommand*>(command)) {
-          flipCommand->loadParams(params);
-          m_pixelsMovement->flipImage(flipCommand->getFlipType());
-          return true;
-        }
-      }
-    }
-  }
 
   // Use StandbyState implementation
   return StandbyState::onKeyDown(editor, msg);
@@ -468,21 +420,69 @@ bool MovingPixelsState::acceptQuickTool(tools::Tool* tool)
 }
 
 // Before executing any command, we drop the pixels (go back to standby).
-void MovingPixelsState::onBeforeCommandExecution(Command* command)
+void MovingPixelsState::onBeforeCommandExecution(CommandExecutionEvent& ev)
 {
+  Command* command = ev.command();
+
+  PRINTF("MovingPixelsState::onBeforeCommandExecution %s\n", command->id().c_str());
+
   // If the command is for other editor, we don't drop pixels.
   if (!isActiveEditor())
     return;
 
   // We don't need to drop the pixels if a MoveMaskCommand of Content is executed.
-  if (MoveMaskCommand* moveMaskCmd = dynamic_cast<MoveMaskCommand*>(command)) {
+  if (MoveMaskCommand* moveMaskCmd = dynamic_cast<MoveMaskCommand*>(ev.command())) {
-    if (moveMaskCmd->getTarget() == MoveMaskCommand::Content)
+    if (moveMaskCmd->getTarget() == MoveMaskCommand::Content) {
+      // Do not drop pixels
       return;
+    }
   }
   else if ((command->id() == CommandId::Zoom) ||
            (command->id() == CommandId::Scroll)) {
+    // Do not drop pixels
     return;
   }
+  // Intercept the "Cut" or "Copy" command to handle them locally
+  // with the current m_pixelsMovement data.
+  else if (command->id() == CommandId::Cut ||
+           command->id() == CommandId::Copy ||
+           command->id() == CommandId::Clear) {
+    // Copy the floating image to the clipboard on Cut/Copy.
+    if (command->id() != CommandId::Clear) {
+      Document* document = m_editor->document();
+      base::UniquePtr<Image> floatingImage;
+      base::UniquePtr<Mask> floatingMask;
+      m_pixelsMovement->getDraggedImageCopy(floatingImage, floatingMask);
+
+      clipboard::copy_image(floatingImage.get(),
+                            floatingMask.get(),
+                            document->sprite()->palette(m_editor->frame()));
+    }
+
+    // Clear floating pixels on Cut/Clear.
+    if (command->id() != CommandId::Copy) {
+      // Discard the dragged image.
+      m_pixelsMovement->discardImage();
+      m_discarded = true;
+
+      // Quit from MovingPixelsState, back to standby.
+      m_editor->backToPreviousState();
+    }
+
+    // Cancel the command, we've simulated it.
+    ev.cancel();
+    return;
+  }
+  // Flip Horizontally/Vertically commands are handled manually to
+  // avoid dropping the floating region of pixels.
+  else if (command->id() == CommandId::Flip) {
+    if (FlipCommand* flipCommand = dynamic_cast<FlipCommand*>(command)) {
+      m_pixelsMovement->flipImage(flipCommand->getFlipType());
+
+      ev.cancel();
+      return;
+    }
+  }
 
   if (m_pixelsMovement)
     dropPixels();
@@ -562,6 +562,8 @@ void MovingPixelsState::setTransparentColor(bool opaque, const app::Color& color
 
 void MovingPixelsState::dropPixels()
 {
+  PRINTF("MovingPixels: drop pixels\n");
+
   // Just change to default state (StandbyState generally). We'll
   // receive an onLeaveState() event after this call.
   m_editor->backToPreviousState();

src/app/ui/editor/moving_pixels_state.h

@@ -22,7 +22,7 @@ namespace doc {
 }
 
 namespace app {
-  class Command;
+  class CommandExecutionEvent;
   class Editor;
 
   class MovingPixelsState
@@ -63,7 +63,7 @@ namespace app {
     void onTransparentColorChange();
 
     // ContextObserver
-    void onBeforeCommandExecution(Command* command);
+    void onBeforeCommandExecution(CommandExecutionEvent& ev);
 
     void setTransparentColor(bool opaque, const app::Color& color);
     void dropPixels();

src/app/ui/editor/play_state.cpp

@@ -148,7 +148,7 @@ void PlayState::onPlaybackTick()
 }
 
 // Before executing any command, we stop the animation
-void PlayState::onBeforeCommandExecution(Command* command)
+void PlayState::onBeforeCommandExecution(CommandExecutionEvent& ev)
 {
   // This check just in case we stay connected to context signals when
   // the editor is already deleted.
@@ -167,9 +167,9 @@ void PlayState::onBeforeCommandExecution(Command* command)
   //
   // There are other commands that just doesn't stop the animation
   // (zoom, scroll, etc.)
-  if (command->id() == CommandId::PlayAnimation ||
+  if (ev.command()->id() == CommandId::PlayAnimation ||
-      command->id() == CommandId::Zoom ||
+      ev.command()->id() == CommandId::Zoom ||
-      command->id() == CommandId::Scroll) {
+      ev.command()->id() == CommandId::Scroll) {
     return;
   }
 

src/app/ui/editor/play_state.h

@@ -16,7 +16,7 @@
 
 namespace app {
 
-  class Command;
+  class CommandExecutionEvent;
 
   class PlayState : public StateWithWheelBehavior {
   public:
@@ -34,7 +34,7 @@ namespace app {
     void onPlaybackTick();
 
     // ContextObserver
-    void onBeforeCommandExecution(Command* command);
+    void onBeforeCommandExecution(CommandExecutionEvent& ev);
 
     double getNextFrameTime();
 

src/app/ui/timeline.cpp

@@ -120,7 +120,8 @@ Timeline::Timeline()
   , m_offset_count(0)
   , m_scroll(false)
 {
-  m_ctxConn = m_context->AfterCommandExecution.connect(&Timeline::onAfterCommandExecution, this);
+  m_ctxConn = m_context->AfterCommandExecution.connect(
+    &Timeline::onAfterCommandExecution, this);
   m_context->documents().addObserver(this);
 
   setDoubleBuffered(true);
@@ -924,7 +925,7 @@ paintNoDoc:;
       skinTheme()->styles.timelinePadding());
 }
 
-void Timeline::onAfterCommandExecution(Command* command)
+void Timeline::onAfterCommandExecution(CommandExecutionEvent& ev)
 {
   if (!m_document)
     return;

src/app/ui/timeline.h

@@ -45,7 +45,7 @@ namespace app {
 
   using namespace doc;
 
-  class Command;
+  class CommandExecutionEvent;
   class ConfigureTimelinePopup;
   class Context;
   class Document;
@@ -113,7 +113,7 @@ namespace app {
     void onSelectionChanged(doc::DocumentEvent& ev) override;
 
     // app::Context slots.
-    void onAfterCommandExecution(Command* command);
+    void onAfterCommandExecution(CommandExecutionEvent& ev);
 
     // DocumentsObserver impl.
     void onRemoveDocument(doc::Document* document) override;