-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add scripting for custom property types #3971
base: master
Are you sure you want to change the base?
Changes from all commits
006572a
f00655b
ab7edce
f30dfdc
185c5ef
a8fb617
17fbc28
fe5c55a
e80e00b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
* scriptpropertytype.cpp | ||
* Copyright 2024, chris <[email protected]> | ||
* | ||
* This file is part of Tiled. | ||
* | ||
* This program is free software; you can redistribute it and/or modify it | ||
* under the terms of the GNU General Public License as published by the Free | ||
* Software Foundation; either version 2 of the License, or (at your option) | ||
* any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for | ||
* more details. | ||
* | ||
* You should have received a copy of the GNU General Public License along with | ||
* this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
#include "scriptpropertytype.h" | ||
|
||
namespace Tiled { | ||
|
||
const QString &ScriptPropertyType::name() const | ||
{ | ||
return mType->name; | ||
} | ||
|
||
void registerPropertyTypes(QJSEngine *jsEngine) | ||
{ | ||
jsEngine->globalObject().setProperty(QStringLiteral("EnumPropertyType"), | ||
jsEngine->newQMetaObject<ScriptEnumPropertyType>()); | ||
jsEngine->globalObject().setProperty(QStringLiteral("ClassPropertyType"), | ||
jsEngine->newQMetaObject<ScriptClassPropertyType>()); | ||
} | ||
|
||
} // namespace Tiled | ||
|
||
#include "moc_scriptpropertytype.cpp" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
/* | ||
* scriptpropertytype.h | ||
* Copyright 2024, chris <[email protected]> | ||
* | ||
* This file is part of Tiled. | ||
* | ||
* This program is free software; you can redistribute it and/or modify it | ||
* under the terms of the GNU General Public License as published by the Free | ||
* Software Foundation; either version 2 of the License, or (at your option) | ||
* any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for | ||
* more details. | ||
* | ||
* You should have received a copy of the GNU General Public License along with | ||
* this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
#pragma once | ||
|
||
#include "propertytype.h" | ||
|
||
#include <QJSEngine> | ||
#include <QList> | ||
#include <QObject> | ||
|
||
namespace Tiled { | ||
/** | ||
* Scripting engine wrapper for PropertyType | ||
*/ | ||
class ScriptPropertyType : public QObject | ||
{ | ||
Q_OBJECT | ||
Q_PROPERTY(QString name READ name) | ||
Q_PROPERTY(bool isClass READ isClass) | ||
Q_PROPERTY(bool isEnum READ isEnum) | ||
Q_PROPERTY(QVariant defaultValue READ defaultValue) | ||
|
||
public: | ||
ScriptPropertyType(const PropertyType *propertyType) | ||
: mType(propertyType) | ||
{} | ||
|
||
const QString &name() const; | ||
bool isClass() const { return mType->isClass(); } | ||
bool isEnum() const { return mType->isEnum(); } | ||
QVariant defaultValue() { return mType->defaultValue(); } | ||
|
||
private: | ||
const PropertyType *mType; | ||
}; | ||
|
||
class ScriptEnumPropertyType : public ScriptPropertyType | ||
{ | ||
Q_OBJECT | ||
|
||
Q_PROPERTY(StorageType storageType READ storageType) | ||
Q_PROPERTY(QStringList values READ values) | ||
|
||
public: | ||
ScriptEnumPropertyType(const EnumPropertyType *propertyType) | ||
: ScriptPropertyType(propertyType) | ||
, mEnumType(propertyType) | ||
{} | ||
// copied from propertytype.h | ||
enum StorageType { | ||
StringValue, | ||
IntValue | ||
}; | ||
Q_ENUM(StorageType); | ||
|
||
StorageType storageType() const { return static_cast<StorageType>(mEnumType->storageType); } | ||
QStringList values() const { return mEnumType->values; } | ||
|
||
private: | ||
const EnumPropertyType *mEnumType; | ||
}; | ||
|
||
class ScriptClassPropertyType : public ScriptPropertyType | ||
{ | ||
Q_OBJECT | ||
Q_PROPERTY(QColor color READ color) | ||
Q_PROPERTY(QVariantMap members READ members) | ||
Q_PROPERTY(bool drawFill READ drawFill) | ||
Q_PROPERTY(int usageFlags READ usageFlags) | ||
|
||
public: | ||
ScriptClassPropertyType(const ClassPropertyType *propertyType) | ||
: ScriptPropertyType(propertyType) | ||
, mClassType(propertyType) | ||
{} | ||
|
||
// TODO: a way to avoid duplicating this again? | ||
enum ClassUsageFlag { | ||
PropertyValueType = 0x001, | ||
|
||
// Keep values synchronized with Object::TypeId | ||
LayerClass = 0x002, | ||
MapObjectClass = 0x004, | ||
MapClass = 0x008, | ||
TilesetClass = 0x010, | ||
TileClass = 0x020, | ||
WangSetClass = 0x040, | ||
WangColorClass = 0x080, | ||
ProjectClass = 0x100, | ||
AnyUsage = 0xFFF, | ||
AnyObjectClass = AnyUsage & ~PropertyValueType, | ||
}; | ||
Q_ENUM(ClassUsageFlag) | ||
|
||
QColor color() const { return mClassType->color; } | ||
// TODO: " No viable overloaded '=' " | ||
// void setColor(QColor &value) { mClassType->color = value; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this is just cause of passing by reference. The argument should either be by value or by const-reference. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neither of those compile right now.
Or
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, it's just because |
||
QVariantMap members() const {return mClassType->members; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a planned feature that would enable manual ordering of class members, so it might be good to expose this as an array instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure whether to make it an array in advance of that feature, but I did want to note that accessing the members through this property doesn't seem to allow adding new members and doesn't handle members of a class type (at least the last time I tried before your push) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean with "doesn't handle members of a class type"? For modification we'll need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd have to test again, but I think when I tested, if a member was a class, it just came back as QVariant and I couldn't do anything with it like get its name or its own nested members There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, you should get a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll test again when I get a chance There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, it's working for properties that are classes 👍
Thanks for fixing the removal of types too . I wonder if removeTypeByName should return a boolean reflecting whether or not the type was found |
||
bool drawFill() const { return mClassType->drawFill; } | ||
// void setDrawFill(bool value) { mClassType->drawFill = value; } | ||
int usageFlags() const { return mClassType->usageFlags; } | ||
//void setUsageFlags(int value) { mClassType->setUsageFlags(value); } | ||
|
||
private: | ||
const ClassPropertyType *mClassType; | ||
}; | ||
|
||
|
||
void registerPropertyTypes(QJSEngine *jsEngine); | ||
|
||
} // namespace Tiled | ||
|
||
Q_DECLARE_METATYPE(Tiled::ScriptPropertyType*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A raw pointer is problematic here, cause there is nothing that makes sure the
ScriptEnumPropertyType
instance is deleted when theEnumPropertyType
instance is deleted. There's a few potential solutions we can try:ScriptEnumPropertyType
from theEnumPropertyType
, so that it can delete the wrapper upon destruction.Project.addType
orProject.setType
to assign a modified object back to the project.I'm not sure at the moment which will be the best way forward. It depends also on the expected usage pattern of the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me the first two seem like the best options. I'd prefer to retain the ability to modify a type's fields just by setting them -- I think that's simpler. So that would mean no option 3. Option 4 seems a bit messy for the reasons you mentioned (we may also want to allow renaming the types from scripting I would think, so that would be complicated by option 4)
For option 2, I guess we would have to check if there already has been a ScriptPropertyEnumType created for an EnumPropertyType before we would create a new one. Also if the main types would hold a pointer to the script wrapper, maybe they should handle creating the script wrapper themselves?
Option 1 also sounds promising but challenging for my skill set . Maybe with some pointers (ha) I could do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if we go for the shared pointer route, I wonder where the code for creating the shared pointers would go, maybe in PropertyTypes ? If we get that working, it would solve the const pointer issue too and allow the setters for color, drawFill, etc. to work