Added ability to add (override) an inherited property

In the previous property editor, overriding happened automatically as
soon as you changed a value. Now that the edit widgets are always
visible, I've instead opted to show a disabled widget for inherited
properties along with a '+' button for adding (overriding) that
property.

I think this helps unintensionally setting a value, and the disabled
state.

Currently the VariantMapProperty does not yet handle a possible change
of property type when a removed property has a different type than the
inherited property.

src/libtiled/object.cpp

@@ -40,6 +40,17 @@ Object::~Object()
     delete mEditable;
 }
 
+const ClassPropertyType *Object::classType() const
+{
+    QString objectClassName = className();
+    if (objectClassName.isEmpty() && typeId() == Object::MapObjectType) {
+        auto mapObject = static_cast<const MapObject*>(this);
+        objectClassName = mapObject->effectiveClassName();
+    }
+
+    return propertyTypes().findClassFor(objectClassName, *this);
+}
+
 /**
  * Returns the value of the property \a name, taking into account that it may
  * be inherited from another object or from the class.
@@ -78,32 +89,36 @@ QVariant Object::resolvedProperty(const QString &name) const
 QVariantMap Object::resolvedProperties() const
 {
     QVariantMap allProperties;
+    Tiled::mergeProperties(allProperties, inheritedProperties());
+    return allProperties;
+}
+
+/**
+ * Computes the inherited properties for this object. This excludes the
+ * properties that are directly set on the object.
+ */
+QVariantMap Object::inheritedProperties() const
+{
+    QVariantMap inheritedProperties;
+
     // Insert properties into allProperties in the reverse order that
     // Object::resolvedProperty searches them, to make sure that the
     // same precedence is maintained.
 
-    QString objectClassName = className();
-    if (objectClassName.isEmpty() && typeId() == Object::MapObjectType) {
-        auto mapObject = static_cast<const MapObject*>(this);
-        objectClassName = mapObject->effectiveClassName();
-    }
-
-    if (auto type = propertyTypes().findClassFor(objectClassName, *this))
-        Tiled::mergeProperties(allProperties, type->members);
+    if (auto type = classType())
+        Tiled::mergeProperties(inheritedProperties, type->members);
 
     if (typeId() == Object::MapObjectType) {
         auto mapObject = static_cast<const MapObject*>(this);
 
         if (const Tile *tile = mapObject->cell().tile())
-            Tiled::mergeProperties(allProperties, tile->properties());
+            Tiled::mergeProperties(inheritedProperties, tile->properties());
 
         if (const MapObject *templateObject = mapObject->templateObject())
-            Tiled::mergeProperties(allProperties, templateObject->properties());
+            Tiled::mergeProperties(inheritedProperties, templateObject->properties());
     }
 
-    Tiled::mergeProperties(allProperties, properties());
-
-    return allProperties;
+    return inheritedProperties;
 }
 
 bool Object::setProperty(const QStringList &path, const QVariant &value)

src/libtiled/object.h

@@ -72,6 +72,8 @@ class TILEDSHARED_EXPORT Object
     const QString &className() const;
     void setClassName(const QString &className);
 
+    const ClassPropertyType *classType() const;
+
     /**
      * Returns the properties of this object.
      */
@@ -111,6 +113,7 @@ class TILEDSHARED_EXPORT Object
 
     QVariant resolvedProperty(const QString &name) const;
     QVariantMap resolvedProperties() const;
+    QVariantMap inheritedProperties() const;
 
     /**
      * Returns the value of the object's \a name property, as a string.

src/tiled/changeproperties.cpp

@@ -164,8 +164,7 @@ void SetProperty::undo()
 
 void SetProperty::redo()
 {
-    const QList<Object*> &objects = mObjects;
-    for (Object *obj : objects)
+    for (Object *obj : std::as_const(mObjects))
         mDocument->setPropertyMember(obj, mPath, mValue);
 }
 
@@ -212,8 +211,7 @@ void RemoveProperty::undo()
 
 void RemoveProperty::redo()
 {
-    const QList<Object*> &objects = mObjects;
-    for (Object *obj : objects)
+    for (Object *obj : std::as_const(mObjects))
         mDocument->removeProperty(obj, mName);
 }
 

src/tiled/colorbutton.cpp

@@ -22,7 +22,6 @@
 
 #include "utils.h"
 
-#include <QColorDialog>
 #include <QEvent>
 #include <QStyle>
 
@@ -68,7 +67,8 @@ void ColorButton::changeEvent(QEvent *e)
 
 void ColorButton::pickColor()
 {
-    const QColor newColor = QColorDialog::getColor(mColor, this);
+    const QColor newColor = QColorDialog::getColor(mColor, this, QString(),
+                                                   mDialogOptions);
     if (newColor.isValid())
         setColor(newColor);
 }
@@ -77,7 +77,7 @@ void ColorButton::updateIcon()
 {
     // todo: fix gray icon in disabled state (consider using opacity, and not using an icon at all)
     setIcon(Utils::colorIcon(mColor, iconSize()));
-    setText(mColor.isValid() ? QString() : tr("Unset"));
+    setText(mColor.isValid() ? QString() : tr("Not set"));
 }
 
 #include "moc_colorbutton.cpp"

src/tiled/colorbutton.h

@@ -21,6 +21,7 @@
 #pragma once
 
 #include <QColor>
+#include <QColorDialog>
 #include <QToolButton>
 
 namespace Tiled {
@@ -41,6 +42,9 @@ class ColorButton : public QToolButton
     QColor color() const { return mColor; }
     void setColor(const QColor &color);
 
+    void setShowAlphaChannel(bool enabled)
+    { mDialogOptions.setFlag(QColorDialog::ShowAlphaChannel, enabled); }
+
 signals:
     void colorChanged(const QColor &color);
 
@@ -52,6 +56,7 @@ class ColorButton : public QToolButton
     void updateIcon();
 
     QColor mColor;
+    QColorDialog::ColorDialogOptions mDialogOptions;
 };
 
 } // namespace Tiled

src/tiled/propertieswidget.cpp

@@ -418,10 +418,9 @@ class VariantMapProperty : public GroupProperty
 public:
     VariantMapProperty(const QString &name, QObject *parent = nullptr)
         : GroupProperty(name, parent)
-    {
-    }
+    {}
 
-    void setValue(const QVariantMap &value);
+    void setValue(const QVariantMap &value, const QVariantMap &suggestions = {});
     const QVariantMap &value() const { return mValue; }
 
 signals:
@@ -444,6 +443,7 @@ class VariantMapProperty : public GroupProperty
     void emitValueChangedRecursively(Property *property);
 
     QVariantMap mValue;
+    QVariantMap mSuggestions;
     QHash<QString, Property*> mPropertyMap;
 };
 
@@ -464,7 +464,31 @@ class CustomProperties : public VariantMapProperty
 
 private:
     // todo: optimize further
+    void onChanged(const ChangeEvent &change) {
+        if (change.type != ChangeEvent::ObjectsChanged)
+            return;
+
+        auto object = mDocument->currentObject();
+        if (!object)
+            return;
+
+        auto &objectsChange = static_cast<const ObjectsChangeEvent&>(change);
+
+        if (objectsChange.properties & ObjectsChangeEvent::ClassProperty) {
+            if (objectsChange.objects.contains(object)) {
+                refresh();
+            } else if (object->typeId() == Object::MapObjectType) {
+                auto mapObject = static_cast<MapObject*>(object);
+                if (auto tile = mapObject->cell().tile()) {
+                    if (mapObject->className().isEmpty() && objectsChange.objects.contains(tile))
+                        refresh();
+                }
+            }
+        }
+    }
     void propertyAdded(Object *object, const QString &) {
+        if (mUpdating)
+            return;
         if (!objectPropertiesRelevant(mDocument, object))
             return;
         refresh();
@@ -1196,6 +1220,7 @@ class ImageLayerProperties : public LayerProperties
                     [this](const QColor &value) {
                         push(new ChangeImageLayerTransparentColor(mapDocument(), { imageLayer() }, value));
                     });
+        mTransparentColorProperty->setAlpha(false);
 
         mRepeatProperty = new ImageLayerRepeatProperty(
                     tr("Repeat"),
@@ -1250,7 +1275,7 @@ class ImageLayerProperties : public LayerProperties
 
     GroupProperty *mImageLayerProperties;
     UrlProperty *mImageProperty;
-    Property *mTransparentColorProperty;
+    ColorProperty *mTransparentColorProperty;
     Property *mRepeatProperty;
 };
 
@@ -2227,19 +2252,24 @@ void PropertiesWidget::currentObjectChanged(Object *object)
 }
 
 
-void VariantMapProperty::setValue(const QVariantMap &value)
+void VariantMapProperty::setValue(const QVariantMap &value,
+                                  const QVariantMap &suggestions)
 {
     mValue = value;
+    mSuggestions = suggestions;
+
+    QVariantMap allProperties = mSuggestions;
+    mergeProperties(allProperties, mValue);
 
     clear();
 
-    QMapIterator<QString, QVariant> it(mValue);
+    QMapIterator<QString, QVariant> it(allProperties);
     while (it.hasNext()) {
         it.next();
         const QString &name = it.key();
 
         auto get = [=] {
-            return mValue.value(name);
+            return mValue.value(name, mSuggestions.value(name));
         };
         auto set = [=] (const QVariant &value) {
             mValue.insert(name, value);
@@ -2248,7 +2278,12 @@ void VariantMapProperty::setValue(const QVariantMap &value)
         };
 
         if (auto property = createProperty({ name }, std::move(get), std::move(set))) {
-            property->setActions(Property::Remove);
+            if (mValue.contains(name)) {
+                property->setActions(Property::Action::Remove);
+            } else {
+                property->setEnabled(false);
+                property->setActions(Property::Action::Add);
+            }
 
             updateModifiedRecursively(property, it.value());
 
@@ -2258,12 +2293,34 @@ void VariantMapProperty::setValue(const QVariantMap &value)
             connect(property, &Property::removeRequested, this, [=] {
                 mValue.remove(name);
 
-                if (auto property = mPropertyMap.take(name))
-                    deleteProperty(property);
+                if (!mSuggestions.contains(name)) {
+                    if (auto property = mPropertyMap.take(name))
+                        deleteProperty(property);
+                } else {
+                    if (auto property = mPropertyMap.value(name)) {
+                        property->setEnabled(false);
+                        property->setActions(Property::Action::Add);
+                        emitValueChangedRecursively(property);
+                        updateModifiedRecursively(property, mSuggestions.value(name));
+                    }
+                }
 
                 emit memberValueChanged({ name }, QVariant());
                 emit valueChanged();
             });
+
+            connect(property, &Property::addRequested, this, [=] {
+                const auto memberValue = mSuggestions.value(name);
+                mValue.insert(name, memberValue);
+
+                if (auto property = mPropertyMap.value(name)) {
+                    property->setEnabled(true);
+                    property->setActions(Property::Action::Remove);
+                }
+
+                emit memberValueChanged({ name }, memberValue);
+                emit valueChanged();
+            });
         }
     }
 
@@ -2364,7 +2421,7 @@ void VariantMapProperty::createClassMembers(const QStringList &path,
         };
 
         if (auto childProperty = createProperty(childPath, std::move(getMember), setMember)) {
-            childProperty->setActions(Property::Reset);
+            childProperty->setActions(Property::Action::Reset);
             groupProperty->addProperty(childProperty);
 
             connect(childProperty, &Property::resetRequested, this, [=] {
@@ -2416,6 +2473,8 @@ void CustomProperties::setDocument(Document *document)
     mDocument = document;
 
     if (document) {
+        connect(document, &Document::changed, this, &CustomProperties::onChanged);
+
         connect(document, &Document::currentObjectsChanged, this, &CustomProperties::refresh);
 
         connect(document, &Document::propertyAdded, this, &CustomProperties::propertyAdded);
@@ -2429,10 +2488,14 @@ void CustomProperties::setDocument(Document *document)
 
 void CustomProperties::refresh()
 {
-    if (mDocument && mDocument->currentObject())
-        setValue(mDocument->currentObject()->properties());
-    else
+    if (!mDocument || !mDocument->currentObject()) {
         setValue({});
+        return;
+    }
+
+    // todo: gather the values from all selected objects
+    setValue(mDocument->currentObject()->properties(),
+             mDocument->currentObject()->inheritedProperties());
 }
 
 void CustomProperties::setPropertyValue(const QStringList &path, const QVariant &value)

src/tiled/propertyeditorwidgets.cpp

@@ -545,14 +545,19 @@ void PropertyLabel::setModified(bool modified)
     setFont(f);
 }
 
-void PropertyLabel::mousePressEvent(QMouseEvent *event)
-{
-    if (m_expandable && event->button() == Qt::LeftButton) {
-        setExpanded(!m_expanded);
-        return;
+bool PropertyLabel::event(QEvent *event)
+{
+    // Handled here instead of in mousePressEvent because we want it to be
+    // expandable also when the label is disabled.
+    if (event->type() == QEvent::MouseButtonPress && m_expandable) {
+        auto mouseEvent = static_cast<QMouseEvent *>(event);
+        if (mouseEvent->button() == Qt::LeftButton) {
+            setExpanded(!m_expanded);
+            return true;
+        }
     }
 
-    ElidingLabel::mousePressEvent(event);
+    return ElidingLabel::event(event);
 }
 
 void PropertyLabel::paintEvent(QPaintEvent *event)

src/tiled/propertyeditorwidgets.h

@@ -302,7 +302,7 @@ class PropertyLabel : public ElidingLabel
     void toggled(bool expanded);
 
 protected:
-    void mousePressEvent(QMouseEvent *event) override;
+    bool event(QEvent *event) override;
     void paintEvent(QPaintEvent *) override;
 
 private:

src/tiled/scriptmodule.cpp

@@ -74,8 +74,8 @@ ScriptModule::ScriptModule(QObject *parent)
 
 ScriptModule::~ScriptModule()
 {
-    for (const auto &pair : mRegisteredActions)
-        ActionManager::unregisterAction(pair.second.get(), pair.first);
+    for (const auto &[id, action] : mRegisteredActions)
+        ActionManager::unregisterAction(action.get(), id);
 
     ActionManager::clearMenuExtensions();