Skip to content

Commit

Permalink
Fixed crash when changing file member of custom class
Browse files Browse the repository at this point in the history
This crash was caused by an infinite recursion happening when changing
top-level FilePath values. It only affected Qt 6 builds, because two
QVariant values that both hold a FilePath value always compared as
unequal due to FilePath not defining a comparision operator.

In addition to adding the comparison operator, I've also added a
condition to the updating of the property value. It should only be
necessary to update the top-level value when a nested value was changed.
Further more, it now sets mUpdatingDetails as another protection against
recursive calls.

For completeness I've also added the comparision operator to
PropertyValue and ObjectRef, though they would not have caused problems
in this context since they're not used as property display values.

Closes #3783
  • Loading branch information
bjorn committed Jul 28, 2023
1 parent c6006d6 commit 2613ed6
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Scripting: Added API for editing tile layers using terrain sets (with a-morphous, #3758)
* Scripting: Support erasing tiles in Tool.preview and TileMap.merge
* Scripting: Added WangSet.effectiveTypeForColor
* Fixed crash when changing file property of custom class (#3783)
* Fixed object preview position with parallax factor on group layer (#3669)
* Fixed hover highlight rendering with active parallax factor (#3669)
* Fixed updating of object selection outlines when changing parallax factor (#3669)
Expand Down
9 changes: 9 additions & 0 deletions src/libtiled/properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ class TILEDSHARED_EXPORT PropertyValue

const PropertyType *type() const;
QString typeName() const;

bool operator==(const PropertyValue &o) const
{ return typeId == o.typeId && value == o.value; }
};

class TILEDSHARED_EXPORT FilePath
Expand All @@ -70,6 +73,9 @@ class TILEDSHARED_EXPORT FilePath
public:
QUrl url;

bool operator==(const FilePath &o) const
{ return url == o.url; }

static QString toString(const FilePath &path);
static FilePath fromString(const QString &string);
};
Expand All @@ -82,6 +88,9 @@ class TILEDSHARED_EXPORT ObjectRef
public:
int id;

bool operator==(const ObjectRef &o) const
{ return id == o.id; }

static int toInt(const ObjectRef &ref) { return ref.id; }
static ObjectRef fromInt(int id) { return ObjectRef { id }; }
};
Expand Down
13 changes: 9 additions & 4 deletions src/tiled/propertytypeseditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1133,13 +1133,18 @@ void PropertyTypesEditor::memberValueChanged(const QStringList &path, const QVar
if (!classType)
return;

auto &topLevelName = path.first();

if (!setPropertyMemberValue(classType->members, path, value))
return;

if (auto property = mPropertiesHelper->property(topLevelName))
property->setValue(mPropertiesHelper->toDisplayValue(classType->members.value(topLevelName)));
// When a nested property was changed, we need to update the value of the
// top-level property to match.
if (path.size() > 1) {
auto &topLevelName = path.first();
if (auto property = mPropertiesHelper->property(topLevelName)) {
QScopedValueRollback<bool> updatingDetails(mUpdatingDetails, true);
property->setValue(mPropertiesHelper->toDisplayValue(classType->members.value(topLevelName)));
}
}

applyPropertyTypes();
}
Expand Down

0 comments on commit 2613ed6

Please sign in to comment.