Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when assigning file property in custom type editor #3783

Closed
robertk92 opened this issue Jul 11, 2023 · 2 comments · Fixed by #3795
Closed

Crash when assigning file property in custom type editor #3783

robertk92 opened this issue Jul 11, 2023 · 2 comments · Fixed by #3795
Assignees
Labels
bug Broken behavior.

Comments

@robertk92
Copy link

The program crashes 5/5 times I've tried this when I assign a value to a file property in the custom type editor. I have verified this in a blank project, same thing happens.

  1. Go to View -> Custom Type Editor.
  2. Create a new type.
  3. Add a file property.
  4. Assign a file to the property by browsing to any file on disk.
  5. The program crashes.

OS: Windows 11
Tiled Version: 1.10.1

@robertk92 robertk92 added the bug Broken behavior. label Jul 11, 2023
@bjorn
Copy link
Member

bjorn commented Jul 27, 2023

Hmm, this is happening due to infinite recursion in PropertyTypesEditor::memberValueChanged, likely a bug introduced in adc443d. I'll have a closer look at it soon.

@bjorn
Copy link
Member

bjorn commented Jul 28, 2023

Turned out that in addition to the above commit property being related, this was also only happening in Qt 6 builds, so it will not have affected official Tiled 1.9 releases.

bjorn added a commit to bjorn/tiled-dev that referenced this issue Jul 28, 2023
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 mapeditor#3783
bjorn added a commit to bjorn/tiled-dev that referenced this issue Jul 28, 2023
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 mapeditor#3783
bjorn added a commit that referenced this issue Jul 28, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants