Stamp Brush right-click cut when Shift is held (#3963)

Implementing this required a refactor of the state system in StampBrush. Behavior and State were previously handled in the same property, which resulted both in wrong behavior when using capture and pressing the behavior keys (replicate by holding right click, start holding shift, release right click, the behavior wasn't set to line as it should have). This commit refactors this system by splitting State and Behavior in two enums and two properties, resulting in making the implemented feature possible, fixing behavior bugs, and future-proofing this system for expansion. Implements #3961 Co-authored-by: Thorbjørn Lindeijer <[email protected]>
mapeditor · Jun 14, 2024 · 9de4b79 · 9de4b79
1 parent 0bc36ee
commit 9de4b79
Show file tree

Hide file tree

Showing 10 changed files with 150 additions and 131 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -3,6 +3,7 @@
 * Added --project command-line parameter for use when exporting (#3797)
 * Added group layer names in "Move Object to Layer" menu (#3454)
 * Added lock icon to open tabs for which the file is read-only
+* Added Shift modifier to cut when capturing a tile stamp (by kdx2a, #3961)
 * Made adding "Copy" when duplicating optional and disabled by default (#3917)
 * Layer names are now trimmed when edited in the UI, to avoid accidental whitespace
 * Scripting: Added API for working with worlds (#3539)

diff --git a/src/tiled/abstracttilefilltool.cpp b/src/tiled/abstracttilefilltool.cpp
@@ -76,7 +76,9 @@ void AbstractTileFillTool::deactivate(MapScene *scene)
 
 void AbstractTileFillTool::mousePressed(QGraphicsSceneMouseEvent *event)
 {
-    if (event->button() == Qt::RightButton && event->modifiers() == Qt::NoModifier) {
+    if (event->button() == Qt::RightButton &&
+        !(event->modifiers() & Qt::ControlModifier))
+    {
         mCaptureStampHelper.beginCapture(tilePosition());
         return;
     }
@@ -89,7 +91,8 @@ void AbstractTileFillTool::mouseReleased(QGraphicsSceneMouseEvent *event)
     if (event->button() == Qt::RightButton && mCaptureStampHelper.isActive()) {
         clearOverlay();
 
-        TileStamp stamp = mCaptureStampHelper.endCapture(*mapDocument(), tilePosition());
+        const bool cut = event->modifiers() & Qt::ShiftModifier;
+        TileStamp stamp = mCaptureStampHelper.endCapture(*mapDocument(), tilePosition(), cut);
         if (!stamp.isEmpty())
             emit stampChanged(stamp);
 

diff --git a/src/tiled/capturestamphelper.cpp b/src/tiled/capturestamphelper.cpp
@@ -38,7 +38,7 @@ void CaptureStampHelper::beginCapture(QPoint tilePosition)
     mCaptureStart = tilePosition;
 }
 
-TileStamp CaptureStampHelper::endCapture(const MapDocument &mapDocument, QPoint tilePosition)
+TileStamp CaptureStampHelper::endCapture(MapDocument &mapDocument, QPoint tilePosition, bool cut)
 {
     mActive = false;
 
@@ -55,6 +55,14 @@ TileStamp CaptureStampHelper::endCapture(const MapDocument &mapDocument, QPoint
                                   captured,
                                   *stamp);
 
+    // Erase captured area when cutting
+    if (cut && !captured.isEmpty()) {
+        const bool allLayers = false;
+        const bool mergeable = false;
+        mapDocument.eraseTileLayers(captured, allLayers, mergeable,
+                                    Document::tr("Cut"));
+    }
+
     if (stamp->layerCount() > 0) {
         stamp->normalizeTileLayerPositionsAndMapSize();
 

diff --git a/src/tiled/capturestamphelper.h b/src/tiled/capturestamphelper.h
@@ -34,7 +34,7 @@ class CaptureStampHelper
     CaptureStampHelper();
 
     void beginCapture(QPoint tilePosition);
-    TileStamp endCapture(const MapDocument &mapDocument, QPoint tilePosition);
+    TileStamp endCapture(MapDocument &mapDocument, QPoint tilePosition, bool cut);
 
     bool isActive() const { return mActive; }
     void reset();

diff --git a/src/tiled/eraser.cpp b/src/tiled/eraser.cpp
@@ -22,11 +22,9 @@
 
 #include "brushitem.h"
 #include "geometry.h"
-#include "map.h"
 #include "mapdocument.h"
 #include "mapscene.h"
 #include "painttilelayer.h"
-#include "tilelayer.h"
 
 #include <QCoreApplication>
 
@@ -111,45 +109,7 @@ void Eraser::doErase(bool continuation)
     }
     mLastTilePos = tilePos;
 
-    QList<QPair<QRegion, TileLayer*>> erasedRegions;
-
-    auto *eraseCommand = new PaintTileLayer(mapDocument());
-    eraseCommand->setText(QCoreApplication::translate("Undo Commands", "Erase"));
-    eraseCommand->setMergeable(continuation);
-
-    auto eraseOnLayer = [&] (TileLayer *tileLayer) {
-        if (!tileLayer->isUnlocked())
-            return;
-
-        QRegion eraseRegion = globalEraseRegion.intersected(tileLayer->bounds());
-        if (eraseRegion.isEmpty())
-            return;
-
-        eraseCommand->erase(tileLayer, eraseRegion);
-
-        erasedRegions.append({ eraseRegion, tileLayer });
-    };
-
-    if (mAllLayers) {
-        for (Layer *layer : mapDocument()->map()->tileLayers())
-            eraseOnLayer(static_cast<TileLayer*>(layer));
-    } else if (!mapDocument()->selectedLayers().isEmpty()) {
-        for (Layer *layer : mapDocument()->selectedLayers())
-            if (TileLayer *tileLayer = layer->asTileLayer())
-                eraseOnLayer(tileLayer);
-    } else if (auto tileLayer = currentTileLayer()) {
-        eraseOnLayer(tileLayer);
-    }
-
-    if (!erasedRegions.isEmpty())
-        mapDocument()->undoStack()->push(eraseCommand);
-
-    for (auto &[region, tileLayer] : std::as_const(erasedRegions)) {
-        if (tileLayer->map() != mapDocument()->map())
-            continue;
-
-        emit mapDocument()->regionEdited(region, tileLayer);
-    }
+    mapDocument()->eraseTileLayers(globalEraseRegion, mAllLayers, continuation);
 }
 
 QRect Eraser::eraseArea() const

diff --git a/src/tiled/mapdocument.cpp b/src/tiled/mapdocument.cpp
@@ -1131,6 +1131,55 @@ void MapDocument::paintTileLayers(const Map &map, bool mergeable,
     }
 }
 
+void MapDocument::eraseTileLayers(const QRegion &region,
+                                  bool allLayers,
+                                  bool mergeable,
+                                  const QString &customName)
+{
+    QList<QPair<QRegion, TileLayer*>> erasedRegions;
+
+    auto eraseCommand = std::make_unique<PaintTileLayer>(this);
+    eraseCommand->setText(customName.isEmpty() ? QCoreApplication::translate("Undo Commands", "Erase")
+                                               : customName);
+    eraseCommand->setMergeable(mergeable);
+
+    auto eraseOnLayer = [&] (TileLayer *tileLayer) {
+        if (!tileLayer->isUnlocked())
+            return;
+
+        QRegion eraseRegion = region.intersected(tileLayer->bounds());
+        if (eraseRegion.isEmpty())
+            return;
+
+        eraseCommand->erase(tileLayer, eraseRegion);
+
+        erasedRegions.append({ eraseRegion, tileLayer });
+    };
+
+    if (allLayers) {
+        for (Layer *layer : map()->tileLayers())
+            eraseOnLayer(static_cast<TileLayer*>(layer));
+    } else if (!selectedLayers().isEmpty()) {
+        for (Layer *layer : selectedLayers())
+            if (TileLayer *tileLayer = layer->asTileLayer())
+                eraseOnLayer(tileLayer);
+    } else if (auto tileLayer = dynamic_cast<TileLayer*>(currentLayer())) {
+        eraseOnLayer(tileLayer);
+    }
+
+    if (!erasedRegions.isEmpty())
+        undoStack()->push(eraseCommand.release());
+
+    for (auto &[region, tileLayer] : std::as_const(erasedRegions)) {
+        // Sanity check needed because a script might respond to the below
+        // signal by removing the layer from the map.
+        if (tileLayer->map() != map())
+            continue;
+
+        emit regionEdited(region, tileLayer);
+    }
+}
+
 void MapDocument::replaceObjectTemplate(const ObjectTemplate *oldObjectTemplate,
                                         const ObjectTemplate *newObjectTemplate)
 {

diff --git a/src/tiled/mapdocument.h b/src/tiled/mapdocument.h
@@ -181,6 +181,10 @@ class TILED_EDITOR_EXPORT MapDocument final : public Document
     void paintTileLayers(const Map &map, bool mergeable = false,
                          QVector<SharedTileset> *missingTilesets = nullptr,
                          QHash<TileLayer *, QRegion> *paintedRegions = nullptr);
+    void eraseTileLayers(const QRegion &region,
+                         bool allLayers = false,
+                         bool mergeable = false,
+                         const QString &customName = QString());
 
     void replaceObjectTemplate(const ObjectTemplate *oldObjectTemplate,
                                const ObjectTemplate *newObjectTemplate);

diff --git a/src/tiled/mapdocumentactionhandler.cpp b/src/tiled/mapdocumentactionhandler.cpp
@@ -465,6 +465,8 @@ void MapDocumentActionHandler::delete_()
     }
 
     for (auto &erased : std::as_const(erasedRegions)) {
+        // Sanity check needed because a script might respond to the below
+        // signal by removing the layer from the map.
         if (erased.second->map() != mMapDocument->map())
             continue;