Skip to content

Commit

Permalink
Fixed display of overridden values from a nested class
Browse files Browse the repository at this point in the history 
When a class A was used as a member of another class B, and its values
were overridden for that member, then instances of class B would not
show the correct values for the members of class A. It would show the
defaults for class A instead.
  • Loading branch information
@bjorn
bjorn committed Sep 9, 2022
1 parent adc443d commit 449f333
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 70 deletions.
1 change: 1 addition & 0 deletions NEWS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Allow adding maps to image collection tilesets (#3447)
* Added file system actions to the tile context menu (#3448)
* Fixed possible crash in Custom Types Editor (#3465)
* Fixed display of overridden values from a nested class
* Fixed ability to reset nested string and file properties (#3409)
* Fixed changing nested property values for multiple objects (#3344)
* Fixed possible duplication of Automapping Rules Tileset (#3462)
Expand Down
105 changes: 39 additions & 66 deletions src/tiled/custompropertieshelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,18 @@ QtVariantProperty *CustomPropertiesHelper::createProperty(const QString &name,
{
Q_ASSERT(!mProperties.contains(name));

QScopedValueRollback<bool> updating(mUpdating, true);

QtVariantProperty *property = createPropertyInternal(name, value);
property->setValue(toDisplayValue(value));

mProperties.insert(name, property);

return property;
}

QtVariantProperty *CustomPropertiesHelper::createPropertyInternal(const QString &name, const QVariant &value)
QtVariantProperty *CustomPropertiesHelper::createPropertyInternal(const QString &name,
const QVariant &value)
{
int type = value.userType();

Expand Down Expand Up @@ -119,11 +124,6 @@ QtVariantProperty *CustomPropertiesHelper::createPropertyInternal(const QString
mPropertyTypeIds.insert(property, 0);
}

// Avoids emitting propertyValueChanged
QScopedValueRollback<bool> initializing(mApplyingToChildren, true);

property->setValue(toDisplayValue(value));

return property;
}

Expand Down Expand Up @@ -193,63 +193,37 @@ void CustomPropertiesHelper::onValueChanged(QtProperty *property, const QVariant
if (!mPropertyTypeIds.contains(property))
return;

if (!mApplyingToChildren) {
if (!mUpdating) {
const auto propertyValue = fromDisplayValue(property, value);
const auto path = propertyPath(property);

// If this is the most nested change
if (!mApplyingToParent) {
const auto path = propertyPath(property);

QScopedValueRollback<bool> updating(mEmittingValueChanged, true);
emit propertyMemberValueChanged(path, propertyValue);
}

if (auto parent = static_cast<QtVariantProperty*>(mPropertyParents.value(property))) {
// Bubble the value up to the parent

auto variantMap = parent->value().toMap();
variantMap.insert(property->propertyName(), propertyValue);

// This might trigger another call of this function, in case of
// recursive class members.
QScopedValueRollback<bool> updating(mApplyingToParent, true);
parent->setValue(variantMap);

property->setModified(!variantMap.isEmpty());
}
QScopedValueRollback<bool> emittingValueChanged(mEmittingValueChanged, true);
emit propertyMemberValueChanged(path, propertyValue);
}

if (!mApplyingToParent) {
if (auto type = propertyType(property); type && type->isClass()) {
// Apply the change to the children

auto &members = static_cast<const ClassPropertyType&>(*type).members;
if (auto type = propertyType(property); type && type->isClass()) {
// Apply the change to the children

const auto subProperties = property->subProperties();
const auto map = value.toMap();
auto &members = static_cast<const ClassPropertyType&>(*type).members;

QScopedValueRollback<bool> updating(mApplyingToChildren, true);
const auto subProperties = property->subProperties();
const auto map = value.toMap();

for (QtProperty *subProperty : subProperties) {
const auto name = subProperty->propertyName();
const bool modified = map.contains(name);
const auto value = modified ? map.value(name)
: members.value(name);
QScopedValueRollback<bool> updating(mUpdating, true);

subProperty->setModified(modified);
for (QtProperty *subProperty : subProperties) {
const auto name = subProperty->propertyName();
const bool modified = map.contains(name);
const auto value = modified ? map.value(name)
: members.value(name);

// Avoid setting child class members as modified, just because
// the class definition sets different defaults on them.
if (!modified) {
auto memberType = propertyType(subProperty);
if (memberType && memberType->isClass()) {
static_cast<QtVariantProperty*>(subProperty)->setValue(QVariantMap());
continue;
}
}
// Avoid setting child class members as modified, just because
// the class definition sets different defaults on them.
const bool isParentTopLevel = !mPropertyParents.contains(property);
const bool isParentModified = property->isModified();
subProperty->setModified(modified && (isParentTopLevel || isParentModified));

static_cast<QtVariantProperty*>(subProperty)->setValue(toDisplayValue(value));
}
static_cast<QtVariantProperty*>(subProperty)->setValue(toDisplayValue(value));
}
}
}
Expand Down Expand Up @@ -292,12 +266,20 @@ void CustomPropertiesHelper::propertyTypesChanged()
if (!typeId)
continue;

if (const auto type = Object::propertyTypes().findTypeById(typeId))
if (const auto type = Object::propertyTypes().findTypeById(typeId)) {
setPropertyAttributes(property, *type);

if (type->isClass()) {
// Restore the existing member values
QScopedValueRollback<bool> updating(mUpdating, true);
onValueChanged(property, property->value());
}
}
}
}

void CustomPropertiesHelper::setPropertyAttributes(QtVariantProperty *property, const PropertyType &propertyType)
void CustomPropertiesHelper::setPropertyAttributes(QtVariantProperty *property,
const PropertyType &propertyType)
{
switch (propertyType.type) {
case Tiled::PropertyType::PT_Invalid:
Expand All @@ -308,23 +290,14 @@ void CustomPropertiesHelper::setPropertyAttributes(QtVariantProperty *property,
// Delete any existing sub-properties
deleteSubProperties(property);

const auto propertyValue = property->value().toMap();

// Create a sub-property for each member
QMapIterator<QString, QVariant> it(classType.members);
while (it.hasNext()) {
it.next();
const QString &name = it.key();
const QVariant &value = it.value();

QtVariantProperty *subProperty = createPropertyInternal(name, value);

if (propertyValue.contains(name)) {
QScopedValueRollback<bool> initializing(mApplyingToChildren, true);
subProperty->setModified(true);
subProperty->setValue(toDisplayValue(propertyValue.value(name)));
}

auto subProperty = createPropertyInternal(name, value);
property->addSubProperty(subProperty);
mPropertyParents.insert(subProperty, property);
}
Expand All @@ -342,7 +315,7 @@ void CustomPropertiesHelper::setPropertyAttributes(QtVariantProperty *property,
}

// Setting these attributes leads to emission of valueChanged...
QScopedValueRollback<bool> suppressValueChanged(mApplyingToChildren, true);
QScopedValueRollback<bool> updating(mUpdating, true);

if (enumType.valuesAsFlags) {
mPropertyManager->setAttribute(property, QStringLiteral("flagNames"), enumType.values);
Expand Down
15 changes: 11 additions & 4 deletions src/tiled/custompropertieshelper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class CustomPropertiesHelper : public QObject
void clear();
bool hasProperty(QtProperty *property) const;
QtVariantProperty *property(const QString &name);
const QHash<QString, QtVariantProperty *> &properties() const;

QVariant toDisplayValue(QVariant value) const;
QVariant fromDisplayValue(QtProperty *property, QVariant value) const;
Expand All @@ -59,15 +60,17 @@ class CustomPropertiesHelper : public QObject
void recreateProperty(QtVariantProperty *property, const QVariant &value);

private:
QtVariantProperty *createPropertyInternal(const QString &name, const QVariant &value);
QtVariantProperty *createPropertyInternal(const QString &name,
const QVariant &value);
void deletePropertyInternal(QtProperty *property);
void deleteSubProperties(QtProperty *property);

void onValueChanged(QtProperty *property, const QVariant &value);
void resetProperty(QtProperty *property);
void propertyTypesChanged();

void setPropertyAttributes(QtVariantProperty *property, const PropertyType &propertyType);
void setPropertyAttributes(QtVariantProperty *property,
const PropertyType &propertyType);

const PropertyType *propertyType(QtProperty *property) const;
QStringList propertyPath(QtProperty *property) const;
Expand All @@ -78,8 +81,7 @@ class CustomPropertiesHelper : public QObject
QHash<QString, QtVariantProperty *> mProperties;
QHash<QtProperty *, int> mPropertyTypeIds;
QHash<QtProperty *, QtProperty *> mPropertyParents;
bool mApplyingToParent = false;
bool mApplyingToChildren = false;
bool mUpdating = false;
bool mEmittingValueChanged = false;
};

Expand All @@ -93,6 +95,11 @@ inline QtVariantProperty *CustomPropertiesHelper::property(const QString &name)
return mProperties.value(name);
}

inline const QHash<QString, QtVariantProperty *> &CustomPropertiesHelper::properties() const
{
return mProperties;
}

inline void CustomPropertiesHelper::setMapDocument(MapDocument *mapDocument)
{
mMapDocument = mapDocument;
Expand Down

0 comments on commit 449f333

Please sign in to comment.