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

Scripting: Fixed UI issues after setting class values #3958

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

bjorn
Copy link
Member

@bjorn bjorn commented Jun 4, 2024

When setting a class value from a script, the value was not getting converted from a JS object to a QVariantMap. This tripped up code elsewhere, for example making the UI unable to edit the values (though it could still display them, and they also got saved correctly).

This change adds the missing conversion from JS object to QVariantMap to tiled.propertyValue and Object.setProperty.

bjorn added 2 commits June 20, 2024 17:51
When setting a class value from a script, the value was not getting
converted from a JS object to a QVariantMap. This tripped up code
elsewhere, for example making the UI unable to edit the values (though
it could still display them, and they also got saved correctly).

This change adds the missing conversion from JS object to QVariantMap to
`tiled.propertyValue` and `Object.setProperty`.
@bjorn bjorn force-pushed the scripting-fix-class-values branch 2 times, most recently from 7cbe772 to 27d64b8 Compare June 20, 2024 15:56
Supported forms are tiled.color(name), which supports for example "red"
or "#ff0000" as color names, or tiled.color(r,g,b,a), with values from
0-1.

Unfortunately, the similar Qt.color and Qt.rgba functions can't be used
because they require the QtQuick module to be loaded (see
QQuickColorProvider in src/quick/util/qquickglobal.cpp). They would work
only when the Tiled extensions were written in QML and imported the
QtQuick module. Tiled could provide its own QQmlColorProvider
implementation, but not without relying on private Qt headers.

Object.setColorProperty is now no longer necessary and has hence been
deprecated.
@bjorn bjorn force-pushed the scripting-fix-class-values branch from 27d64b8 to b5f85a9 Compare June 20, 2024 16:08
@bjorn bjorn merged commit b5f85a9 into mapeditor:master Jun 21, 2024
17 checks passed
@bjorn bjorn deleted the scripting-fix-class-values branch June 21, 2024 09:47
@bjorn
Copy link
Member Author

bjorn commented Jun 21, 2024

Even if this PR adds a convenience function Object.setProperty, I've passed on adding an equivalent Object.property overload that takes a path for now. This functionality doesn't exist on the C++ side either at the moment, so it would be a larger patch, and the usefulness is less obvious since it just makes this sequence a bit less verbose:

// currently supported
let nestedPropertyValue = object.property("foo").value.bar.value.baz;

// vs:
let nestedPropertyValue = object.property(["foo", "bar", "baz"]);

In addition, we'd then also need to add path support to Object.resolvedProperty.

There was no need for overloading Object.setColorProperty or Object.setFloatProperty, because class members have their type defined by their class. The setProperty overload taking a path will try to convert the values to the type of the class member (though this currently isn't the case of top-level properties).

Object.setColorProperty has been deprecated in favor of using the new tiled.color to create a color value. Unfortunately I couldn't immediately find a quick way to do the same with Object.setFloatProperty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant