Fixed issues with importing of Object Types

* Don't try to adjust members when importing Object Types, since they
  will already be referencing current property types (fixed crash).

* Avoid naming conflicts by separating the lookup of types used as
  property values from types used as classes. Two types are now allowed
  to have the same name, as long as their usage context does not overlap
  (though for now only when importing, the Property Types Editor does
  not allow this).

* Fixed test_properties autotest.

src/libtiled/mapobject.cpp

@@ -293,9 +293,8 @@ QColor MapObject::effectiveColor() const
     const QString &effectiveClass = this->effectiveClassName();
 
     // See if this object's class has a color associated with it
-    if (auto type = Object::propertyTypes().findClassByName(effectiveClass))
-        if (type->isClassFor(*this))
-            return type->color;
+    if (auto type = Object::propertyTypes().findClassFor(effectiveClass, *this))
+        return type->color;
 
     // If not, get color from object group
     if (mObjectGroup && mObjectGroup->color().isValid())

src/libtiled/object.cpp

@@ -74,9 +74,8 @@ QVariant Object::resolvedProperty(const QString &name) const
     }
 
     if (!objectClassName.isEmpty()) {
-        if (auto type = mPropertyTypes->findClassByName(objectClassName))
-            if (type->isClassFor(*this))
-                return type->members.value(name);
+        if (auto type = mPropertyTypes->findClassFor(objectClassName, *this))
+            return type->members.value(name);
     }
 
     return QVariant();
@@ -96,11 +95,9 @@ QVariantMap Object::resolvedProperties() const
     }
 
     if (!objectClassName.isEmpty()) {
-        if (auto type = mPropertyTypes->findClassByName(objectClassName)) {
-            if (type->isClassFor(*this)) {
-                Tiled::mergeProperties(allProperties,
-                                       static_cast<const ClassPropertyType*>(type)->members);
-            }
+        if (auto type = mPropertyTypes->findClassFor(objectClassName, *this)) {
+            Tiled::mergeProperties(allProperties,
+                                   static_cast<const ClassPropertyType*>(type)->members);
         }
     }
 

src/libtiled/object.h

@@ -28,7 +28,6 @@
 
 #pragma once
 
-#include "objecttypes.h"
 #include "properties.h"
 #include "propertytype.h"
 

src/libtiled/objecttypes.cpp

@@ -307,25 +307,6 @@ bool ObjectTypesSerializer::readObjectTypes(const QString &fileName,
     return mError.isEmpty();
 }
 
-/**
- * Converts object types to class property types, for compatibility.
- */
-PropertyTypes toPropertyTypes(const ObjectTypes &objectTypes)
-{
-    PropertyTypes propertyTypes;
-
-    // Convert object types to class property types
-    for (const ObjectType &type : qAsConst(objectTypes)) {
-        auto propertyType = std::make_unique<ClassPropertyType>(type.name);
-        propertyType->color = type.color;
-        propertyType->members = type.defaultProperties;
-        propertyType->usageFlags = ClassPropertyType::MapObjectClass | ClassPropertyType::TileClass;
-        propertyTypes.add(std::move(propertyType));
-    }
-
-    return propertyTypes;
-}
-
 /**
  * Converts class property types to object types, for compatibility.
  */

src/libtiled/objecttypes.h

@@ -61,7 +61,6 @@ struct ObjectType
 using ObjectTypes = QVector<ObjectType>;
 
 
-
 class TILEDSHARED_EXPORT ObjectTypesSerializer
 {
 public:
@@ -90,7 +89,6 @@ class TILEDSHARED_EXPORT ObjectTypesSerializer
 TILEDSHARED_EXPORT QJsonArray toJson(const ObjectTypes &objectTypes, const ExportContext &context);
 TILEDSHARED_EXPORT void fromJson(const QJsonArray &array, ObjectTypes &objectTypes, const ExportContext &context);
 
-TILEDSHARED_EXPORT PropertyTypes toPropertyTypes(const ObjectTypes &objectTypes);
 TILEDSHARED_EXPORT ObjectTypes toObjectTypes(const PropertyTypes &propertyTypes);
 
 } // namespace Tiled

src/libtiled/properties.cpp

@@ -251,7 +251,7 @@ QVariant ExportContext::toPropertyValue(const ExportValue &exportValue) const
 
     // Wrap the value in its custom property type when applicable
     if (!exportValue.propertyTypeName.isEmpty()) {
-        if (const PropertyType *propertyType = mTypes.findTypeByName(exportValue.propertyTypeName)) {
+        if (const PropertyType *propertyType = mTypes.findPropertyValueType(exportValue.propertyTypeName)) {
             propertyValue = propertyType->toPropertyValue(propertyValue, *this);
         } else {
            Tiled::ERROR(QStringLiteral("Unrecognized property type: '%1'")

src/libtiled/propertytype.cpp

@@ -31,6 +31,7 @@
 #include "containerhelpers.h"
 #include "logginginterface.h"
 #include "object.h"
+#include "objecttypes.h"
 #include "properties.h"
 
 #include <QVector>
@@ -368,6 +369,10 @@ void ClassPropertyType::initializeFromJson(const QJsonObject &json)
             if (useAsArray.contains(entry.name))
                 usageFlags |= entry.flag;
         }
+    } else {
+        // Before "useAs" was introduced, class types were only used as
+        // property values.
+        usageFlags = PropertyValueType;
     }
 }
 
@@ -385,7 +390,7 @@ void ClassPropertyType::resolveDependencies(const ExportContext &context)
 
         // Remove any members that would result in a circular reference
         if (!exportValue.propertyTypeName.isEmpty()) {
-            if (auto propertyType = context.types().findTypeByName(exportValue.propertyTypeName)) {
+            if (auto propertyType = context.types().findPropertyValueType(exportValue.propertyTypeName)) {
                 if (!canAddMemberOfType(propertyType, context.types())) {
                     Tiled::ERROR(QStringLiteral("Removed member '%1' from class '%2' since it would cause a circular reference")
                                  .arg(it.key(), name));
@@ -456,6 +461,12 @@ size_t PropertyTypes::count(PropertyType::Type type) const
     });
 }
 
+static int typeUsageFlags(const PropertyType &type)
+{
+    return type.isClass() ? static_cast<const ClassPropertyType&>(type).usageFlags
+                          : ClassPropertyType::PropertyValueType;
+}
+
 void PropertyTypes::merge(PropertyTypes typesToMerge)
 {
     QHash<int, QString> oldTypeIdToName;
@@ -466,8 +477,11 @@ void PropertyTypes::merge(PropertyTypes typesToMerge)
 
     while (typesToMerge.count() > 0) {
         auto typeToImport = typesToMerge.takeAt(0);
+        auto typeToImportUsageFlags = typeUsageFlags(*typeToImport);
         auto existingIt = std::find_if(mTypes.begin(), mTypes.end(), [&] (const PropertyType *type) {
-            return type->name == typeToImport->name;
+            // Consider same type only when name matches and usage flags overlap
+            return type->name == typeToImport->name &&
+                    (typeUsageFlags(*type) & typeToImportUsageFlags) != 0;
         });
 
         if (typeToImport->isClass())
@@ -494,7 +508,7 @@ void PropertyTypes::merge(PropertyTypes typesToMerge)
                 PropertyValue classMemberValue = classMember.value<PropertyValue>();
 
                 const QString typeName = oldTypeIdToName.value(classMemberValue.typeId);
-                auto type = findTypeByName(typeName);
+                auto type = findPropertyValueType(typeName);
                 Q_ASSERT(type);
 
                 if (classMemberValue.typeId != type->id) {
@@ -506,6 +520,30 @@ void PropertyTypes::merge(PropertyTypes typesToMerge)
     }
 }
 
+void PropertyTypes::mergeObjectTypes(const QVector<ObjectType> &objectTypes)
+{
+    for (const ObjectType &type : objectTypes) {
+        auto propertyType = std::make_unique<ClassPropertyType>(type.name);
+        propertyType->color = type.color;
+        propertyType->members = type.defaultProperties;
+        propertyType->usageFlags = ClassPropertyType::MapObjectClass | ClassPropertyType::TileClass;
+
+        auto existingIt = std::find_if(mTypes.begin(), mTypes.end(), [&] (const PropertyType *type) {
+            // Consider same type only when name matches and usage flags overlap
+            return type->name == propertyType->name &&
+                    (typeUsageFlags(*type) & propertyType->usageFlags) != 0;
+        });
+
+        if (existingIt != mTypes.end()) {
+            // Replace existing classes, but retain their ID
+            propertyType->id = (*existingIt)->id;
+            delete std::exchange(*existingIt, propertyType.release());
+        } else {
+            add(std::move(propertyType));
+        }
+    }
+}
+
 /**
  * Returns a pointer to the PropertyType matching the given \a typeId, or
  * nullptr if it can't be found.
@@ -519,21 +557,26 @@ const PropertyType *PropertyTypes::findTypeById(int typeId) const
 }
 
 /**
- * Returns a pointer to the PropertyType matching the given \a name, or
- * nullptr if it can't be found.
+ * Returns a pointer to the PropertyType matching the given \a name and
+ * \a usageFlags, or nullptr if it can't be found.
  */
-const PropertyType *PropertyTypes::findTypeByName(const QString &name) const
+const PropertyType *PropertyTypes::findTypeByName(const QString &name, int usageFlags) const
 {
     auto it = std::find_if(mTypes.begin(), mTypes.end(), [&] (const PropertyType *type) {
-        return type->name == name;
+        return type->name == name && (typeUsageFlags(*type) & usageFlags) != 0;
     });
     return it == mTypes.end() ? nullptr : *it;
 }
 
-const ClassPropertyType *PropertyTypes::findClassByName(const QString &name) const
+const PropertyType *PropertyTypes::findPropertyValueType(const QString &name) const
+{
+    return findTypeByName(name, ClassPropertyType::PropertyValueType);
+}
+
+const ClassPropertyType *PropertyTypes::findClassFor(const QString &name, const Object &object) const
 {
     auto it = std::find_if(mTypes.begin(), mTypes.end(), [&] (const PropertyType *type) {
-        return type->name == name && type->isClass();
+        return type->name == name && type->isClass() && static_cast<const ClassPropertyType*>(type)->isClassFor(object);
     });
     return static_cast<const ClassPropertyType*>(it == mTypes.end() ? nullptr : *it);
 }

src/libtiled/propertytype.h

@@ -47,6 +47,7 @@ namespace Tiled {
 class ExportContext;
 class Object;
 class PropertyTypes;
+struct ObjectType;
 
 class TILEDSHARED_EXPORT ExportValue
 {
@@ -148,12 +149,13 @@ class TILEDSHARED_EXPORT ClassPropertyType final : public PropertyType
         WangSetClass        = 0x40,
         WangColorClass      = 0x80,
 
-        AnyObjectClass      = 0xFF & ~PropertyValueType,
+        AnyUsage            = 0xFF,
+        AnyObjectClass      = AnyUsage & ~PropertyValueType,
     };
 
     QVariantMap members;
     QColor color = Qt::gray;
-    int usageFlags = PropertyValueType | AnyObjectClass;
+    int usageFlags = AnyUsage;
 
     ClassPropertyType(const QString &name) : PropertyType(PT_Class, name) {}
 
@@ -199,10 +201,12 @@ class TILEDSHARED_EXPORT PropertyTypes
     PropertyType &typeAt(int index);
     void moveType(int from, int to);
     void merge(PropertyTypes types);
+    void mergeObjectTypes(const QVector<ObjectType> &objectTypes);
 
     const PropertyType *findTypeById(int typeId) const;
-    const PropertyType *findTypeByName(const QString &name) const;
-    const ClassPropertyType *findClassByName(const QString &name) const;
+    const PropertyType *findTypeByName(const QString &name, int usageFlags = ClassPropertyType::AnyUsage) const;
+    const PropertyType *findPropertyValueType(const QString &name) const;
+    const ClassPropertyType *findClassFor(const QString &name, const Object &object) const;
 
     void loadFromJson(const QJsonArray &list, const QString &path = QString());
     QJsonArray toJson(const QString &path = QString()) const;

src/tiled/exporthelper.cpp

@@ -164,10 +164,8 @@ void ExportHelper::resolveTypeAndProperties(MapObject *object) const
 
     // Inherit properties from the class
     if (!object->className().isEmpty()) {
-        if (auto type = Object::propertyTypes().findClassByName(object->className())) {
-            if (type->isClassFor(*object))
-                mergeProperties(properties, static_cast<const ClassPropertyType*>(type)->members);
-        }
+        if (auto type = Object::propertyTypes().findClassFor(object->className(), *object))
+            mergeProperties(properties, static_cast<const ClassPropertyType*>(type)->members);
     }
 
     // Inherit properties from the tile

src/tiled/projectmanager.cpp

@@ -21,6 +21,7 @@
 #include "projectmanager.h"
 
 #include "fileformat.h"
+#include "objecttypes.h"
 #include "preferences.h"
 #include "projectmodel.h"
 
@@ -52,7 +53,7 @@ void ProjectManager::setProject(Project _project)
                                     QFileInfo(project.mObjectTypesFile).path());
 
         if (ObjectTypesSerializer().readObjectTypes(project.mObjectTypesFile, objectTypes, context)) {
-            project.propertyTypes()->merge(toPropertyTypes(objectTypes));
+            project.propertyTypes()->mergeObjectTypes(objectTypes);
             project.mObjectTypesFile.clear();
         }
     }

src/tiled/propertybrowser.cpp

@@ -1933,7 +1933,8 @@ void PropertyBrowser::updateCustomProperties()
 
     mCustomPropertiesHelper.clear();
 
-    mCombinedProperties = mObject->properties();
+    Properties combinedProperties = mObject->properties();
+
     // Add properties from selected objects which mObject does not contain to mCombinedProperties.
     const auto currentObjects = mDocument->currentObjects();
     for (Object *obj : currentObjects) {
@@ -1944,24 +1945,24 @@ void PropertyBrowser::updateCustomProperties()
 
         while (it.hasNext()) {
             it.next();
-            if (!mCombinedProperties.contains(it.key()))
-                mCombinedProperties.insert(it.key(), QString());
+            if (!combinedProperties.contains(it.key()))
+                combinedProperties.insert(it.key(), QString());
         }
     }
 
-    QString objectType = mObject->className();
+    QString className = mObject->className();
 
     if (mObject->typeId() == Object::MapObjectType) {
         auto mapObject = static_cast<MapObject*>(mObject);
-        objectType = mapObject->effectiveClassName();
+        className = mapObject->effectiveClassName();
 
         // Inherit properties from the template
         if (const MapObject *templateObject = mapObject->templateObject()) {
             QMapIterator<QString,QVariant> it(templateObject->properties());
             while (it.hasNext()) {
                 it.next();
-                if (!mCombinedProperties.contains(it.key()))
-                    mCombinedProperties.insert(it.key(), it.value());
+                if (!combinedProperties.contains(it.key()))
+                    combinedProperties.insert(it.key(), it.value());
             }
         }
 
@@ -1970,27 +1971,25 @@ void PropertyBrowser::updateCustomProperties()
             QMapIterator<QString,QVariant> it(tile->properties());
             while (it.hasNext()) {
                 it.next();
-                if (!mCombinedProperties.contains(it.key()))
-                    mCombinedProperties.insert(it.key(), it.value());
+                if (!combinedProperties.contains(it.key()))
+                    combinedProperties.insert(it.key(), it.value());
             }
         }
     }
 
-    if (!objectType.isEmpty()) {
+    if (!className.isEmpty()) {
         // Inherit properties from the object type
-        if (auto type = Object::propertyTypes().findClassByName(objectType)) {
-            if (type->isClassFor(*mObject)) {
-                QMapIterator<QString,QVariant> it(static_cast<const ClassPropertyType*>(type)->members);
-                while (it.hasNext()) {
-                    it.next();
-                    if (!mCombinedProperties.contains(it.key()))
-                        mCombinedProperties.insert(it.key(), it.value());
-                }
+        if (auto type = Object::propertyTypes().findClassFor(className, *mObject)) {
+            QMapIterator<QString,QVariant> it(type->members);
+            while (it.hasNext()) {
+                it.next();
+                if (!combinedProperties.contains(it.key()))
+                    combinedProperties.insert(it.key(), it.value());
             }
         }
     }
 
-    QMapIterator<QString,QVariant> it(mCombinedProperties);
+    QMapIterator<QString,QVariant> it(combinedProperties);
 
     while (it.hasNext()) {
         it.next();

src/tiled/propertybrowser.h

@@ -228,8 +228,6 @@ class PropertyBrowser : public QtTreePropertyBrowser
     QHash<PropertyId, QtVariantProperty *> mIdToProperty;
     CustomPropertiesHelper mCustomPropertiesHelper;
 
-    Properties mCombinedProperties;
-
     QStringList mStaggerAxisNames;
     QStringList mStaggerIndexNames;
     QStringList mOrientationNames;