Adds option to lock/unlock layer #1627

ketanhwr · 2017-06-26T12:47:40Z

Iterative PR for #734.

bjorn

This is starting to look quite nice!

I think there will need to be some better user-feedback though. I noticed GIMP displays a warning in the status bar that reads "The active layer's pixels are locked" when you try to modify a layer. But this is something that can be added later. At least I'm not entirely sure how that would best be done right now.

Another thing, which is also needed for the visibility, is a way to indicate "this layer is locked because a parent is locked", and "this layer is hidden because a parent is hidden". I think we can use the Qt::PartiallyChecked for this and of course we'd need to find an appropriate icon. A simple idea would be to render a faded out icon in this case. Anyway, if you prefer we can also leave that for another change.

In terms of functionality, it seems the main thing remaining are the object layer tools. The user should not be able to select objects on locked layers. This check will need to happen for each object, since the tool in general will be enabled and there can be objects from other layers that it will interact with. All the object insertion tools should just check the locked state.

Finally, I think also the "Offset Layers" tool should refuse to work on a locked layer.

bjorn · 2017-06-27T07:12:07Z

src/tiled/bucketfilltool.cpp

@@ -207,7 +207,7 @@ void BucketFillTool::mousePressed(QGraphicsSceneMouseEvent *event)
        return;

    const TileLayer *preview = mFillOverlay.data();
-    if (!preview)
+    if (!preview || !currentTileLayer()->isUnlocked())


Please do the locked check separately before the preview check.

bjorn · 2017-06-27T07:12:22Z

src/tiled/terrainbrush.cpp

@@ -225,7 +225,7 @@ void TerrainBrush::doPaint(bool mergeable)
    TileLayer *tileLayer = currentTileLayer();
    Q_ASSERT(tileLayer);

-    if (!tileLayer->bounds().intersects(stamp->bounds()))
+    if (!tileLayer->bounds().intersects(stamp->bounds()) || !tileLayer->isUnlocked())


Please do the locked check separately before the bounds check.

bjorn · 2017-06-27T07:17:27Z

src/tiled/iconcheckdelegate.h

 {
 public:
-    explicit EyeVisibilityDelegate(QObject *parent = nullptr);
+    explicit IconCheckDelegate(QObject *parent = nullptr, bool lock = false);


I think it's alright to support lock/visible as an option rather than taking two QIcon parameters, but it should not be a boolean. You could instead use:

enum IconType { VisibilityIcon, LockedIcon };

Also, please always make the parent the last parameter in the list.

bjorn · 2017-06-27T07:18:52Z

src/tiled/layermodel.cpp

-                                                        visible);
-            mMapDocument->undoStack()->push(command);
+        if (index.column() == 1)
+        {


Coding style: { should be on the same line.

bjorn · 2017-06-27T07:45:48Z

src/tiled/layerdock.cpp

+    if (mapDocument) {
+        mLayerView->header()->setSectionResizeMode(0, QHeaderView::Stretch);
+        mLayerView->header()->setSectionResizeMode(1, QHeaderView::ResizeToContents);
+        mLayerView->header()->setSectionResizeMode(2, QHeaderView::ResizeToContents);


Currently, this makes these sections much too wide because the header text, even though hidden, is still taken into account. But even when making that empty, there seems to be some space allocated in addition to the icon. I think we can just set a fixed size on these sections:

mLayerView->header()->setSectionResizeMode(1, QHeaderView::Fixed); mLayerView->header()->setSectionResizeMode(2, QHeaderView::Fixed); mLayerView->header()->resizeSection(1, Utils::dpiScaled(22)); mLayerView->header()->resizeSection(2, Utils::dpiScaled(22));

bjorn · 2017-06-27T07:47:16Z

src/tiled/layermodel.cpp

+                return mImageLayerIcon;
+            case Layer::GroupLayerType:
+                return QApplication::style()->standardIcon(QStyle::SP_DirIcon);
+            }
        }


Missing break; here too.

bjorn · 2017-06-27T07:47:50Z

src/tiled/iconcheckdelegate.cpp

+}
+
+void IconCheckDelegate::drawCheck(QPainter *painter, const QStyleOptionViewItem &,
+                                      const QRect &rect, Qt::CheckState state) const


Indentation is off.

bjorn

Comments on recent changes.

bjorn · 2017-06-27T08:28:27Z

src/tiled/createobjecttool.cpp

@@ -195,7 +195,7 @@ void CreateObjectTool::finishNewMapObject()
    Q_ASSERT(mNewMapObjectItem);

    ObjectGroup *objectGroup = currentObjectGroup();
-    if (!objectGroup) {
+    if (!objectGroup || !objectGroup->isUnlocked()) {


The check is too late at this point. It should not be possible to start the creation of a new object.

bjorn · 2017-06-27T08:32:21Z

src/tiled/mapdocument.cpp

@@ -586,6 +586,8 @@ void MapDocument::moveLayerDown(Layer *layer)
 void MapDocument::removeLayer(Layer *layer)
 {
    Q_ASSERT(layer->map() == mMap);
+    if (!layer->isUnlocked())
+        return;


You should be able to remove locked layers. Also this check would be in the wrong place since this function is not expected to fail.

Locked layer can be removed? I doubt this..

bjorn

It seems only the edit polygon tool remains, right?

I found one issue with the layer offset tool.

bjorn · 2017-06-27T13:46:31Z

src/tiled/layeroffsettool.cpp

@@ -136,7 +136,8 @@ void LayerOffsetTool::startDrag(const QPointF &pos)
    if (!mapDocument())
        return;

-    if (Layer *layer = mapDocument()->currentLayer()) {
+    Layer *layer = mapDocument()->currentLayer();
+    if (layer && layer->isUnlocked()) {


I noticed that this change is not enough, since after calling startDrag, the mouseMoved function continues to change the layer position even if mDragging is not set to true. So I would suggest to remove the return in the else in that function and instead check mDragging in addition to currentLayer before moving the layer.

* Fixed crash when editing collision when tile image wasn't loaded When opening a tileset it can happen that the tileset image fails to load. In this case, opening the tile collision editor could lead to a crash. * GmxPlugin: Fixed tile type inheritance for tile objects Now tile objects of which the tile has a type defined are exported as instances of this type of object in the GameMaker room file. * Added toolbar for stamp brush and bucket fill tool (mapeditor#1586) This adds a new tool-specific toolbar that can be used by tools. Currently contains actions for rotating/flipping stamp and toggling random mode. Closes mapeditor#1084 * docs: Fixed link to other page * QtPropertyBrowser: Removed deprecation warnings The classes were deprecated in Qt 5.0 and warnings have been added in Qt 5.7. * Fixed rendering of tile object outlines for resized objects They were taking the size of the image instead of the size of the object, which means this was broken since Tiled 0.12. * Fixed rendering of tile objects when the image couldn't be loaded If the tile was found but its image failed to load, tile objects would not render at all and due to a broken boundingRect be also impossible to interact with. Now they render as the special "missing image marker" and can be interacted with. * More fixes for labels of objects nested in a group layer * Fixed labels shown on objects hidden via a group layer * Fixed updating of label visibility when toggling group layer visibility * Fixed updating of label positions when moving a group layer When moving a group layer, any labels present for objects nested within that group layer need to be synchronized. * GmxPlugin: Added support for defining views with objects (mapeditor#1621) * Fixed hang when trying to fill with a pasted stamp Since 688ec7d the size of a copied map is set to 0x0 instead of matching the tile layer's size. It was supposed to be irrelevant, but as it turns out TileStamp::maxSize was based on the size of the map instead of the size of the tile layer. This could lead to an infinite loop in fillWithStamp in bucketfilltool.cpp. Closes mapeditor#1617 Closes mapeditor#1624 * Restored Ctrl+N shortcut on "New Map" action There isn't really a good reason not to have this shortcut. Eventually it may pop up a dialog where you can pick what you want to create, but since it's more common to create new maps than new tilesets we can just do that for now. * Use initializer list for quick-stamp keys * Introduced TilesetDocumentsModel and its sort-filter model companion This model lists the tileset documents that are currently open, and the sort-filter version sorts them by name and filters out tilesets that are embedded in other maps than the current one. This model extracts part of the logic from TilesetDock, so that it could be reused by an updated TerrainModel. The TerrainModel currently only lists terrains from tilesets that are already part of the map, but it should display all loaded external tilesets. * libtiled-java: Fixed wrong exception being caught in TileSet (mapeditor#1629) * Display all tilesets with terrain in the Terrains view Except for tilesets that are embedded into another map than the current one, the Terrains view now displays all tilesets that have terrains defined. The Terrain Brush will now automatically add the tileset of the currently selected terrain to the map when it isn't already present. * Show custom properties on tiles and terrains in the map editor While still not editable, this change shows these properties in a read-only fashion. It is often useful to see them, as indicated by multiple users on the forum. * Bumped version to 1.0.2 and updated NEWS file * Adds option to lock/unlock layer (mapeditor#1627) Locking a layer prevents modifications to the layer by the tools, as well as by some actions like cut and delete. Modifications to objects are prevented by making them not selectable. Closes mapeditor#734 * Fixed tool tips on flipping and rotating stamp actions

ketanhwr added 4 commits June 26, 2017 18:02

Adds option to lock/unlock layer

cbf931b

Small changes

abadf63

Shifts visibility toggle to new column

b74f8a9

Adds functionality

d0076fe

bjorn requested changes Jun 27, 2017

View reviewed changes

ketanhwr added 2 commits June 27, 2017 13:43

Adds check to few more functions

90c1b2a

Requested changes

01dc974

bjorn requested changes Jun 27, 2017

View reviewed changes

ketanhwr added 3 commits June 27, 2017 14:19

Minor changes

90dd8c6

Adds check in object selection tool

f7e7b6a

Fixes segmentation fault

230e7ed

bjorn requested changes Jun 27, 2017

View reviewed changes

ketanhwr and others added 3 commits June 27, 2017 19:41

Adds layer locking in edit polygon

22055f9

Bug fix

d54452e

Fixed undo command name

9286444

bjorn merged commit cf1ea29 into mapeditor:master Jun 27, 2017

boaromayo mentioned this pull request Aug 3, 2017

Update forked repo (#1) #1675

Closed

bjorn mentioned this pull request Feb 7, 2023

AutoMapping: Skip locked layers when applying rules #3562

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds option to lock/unlock layer #1627

Adds option to lock/unlock layer #1627

ketanhwr commented Jun 26, 2017

bjorn left a comment

bjorn Jun 27, 2017

bjorn Jun 27, 2017

bjorn Jun 27, 2017

bjorn Jun 27, 2017

bjorn Jun 27, 2017

bjorn Jun 27, 2017

bjorn Jun 27, 2017

bjorn left a comment

bjorn Jun 27, 2017

bjorn Jun 27, 2017

ketanhwr Jun 27, 2017

bjorn left a comment

bjorn Jun 27, 2017

Adds option to lock/unlock layer #1627

Adds option to lock/unlock layer #1627

Conversation

ketanhwr commented Jun 26, 2017

bjorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment