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

Add scripting for custom property types #3971

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
16 changes: 16 additions & 0 deletions src/libtiled/propertytype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,22 @@ void PropertyTypes::mergeObjectTypes(const QVector<ObjectType> &objectTypes)
}
}

/**
* Returns the index of the type of the given name, or -1 if no such type is
* found.
*/
int PropertyTypes::findIndexByName(const QString &name) const
{
if (name.isEmpty())
return -1;

for (int i = 0; i < mTypes.count(); i++)
if (mTypes[i]->name == name)
return i;

return -1;
}

/**
* Returns a pointer to the PropertyType matching the given \a typeId, or
* nullptr if it can't be found.
Expand Down
2 changes: 2 additions & 0 deletions src/libtiled/propertytype.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ class TILEDSHARED_EXPORT PropertyTypes
void merge(PropertyTypes types);
void mergeObjectTypes(const QVector<ObjectType> &objectTypes);

int findIndexByName(const QString &name) const;

const PropertyType *findTypeById(int typeId) const;
const PropertyType *findTypeByName(const QString &name, int usageFlags = ClassPropertyType::AnyUsage) const;
const PropertyType *findPropertyValueType(const QString &name) const;
Expand Down
56 changes: 54 additions & 2 deletions src/tiled/editableproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@

#include "editableproject.h"

#include "preferences.h"
#include "projectdocument.h"
#include "projectmanager.h"

namespace Tiled {

Expand All @@ -31,6 +33,11 @@ EditableProject::EditableProject(ProjectDocument *projectDocument, QObject *pare
setDocument(projectDocument);
}

bool EditableProject::isReadOnly() const
{
return false;
}

QString EditableProject::extensionsPath() const
{
return project()->mExtensionsPath;
Expand All @@ -51,9 +58,12 @@ QStringList EditableProject::folders() const
return project()->folders();
}

bool EditableProject::isReadOnly() const
QVector<ScriptPropertyType *>EditableProject::propertyTypes() const
{
return false;
QVector<ScriptPropertyType*> scriptTypes;
for (const PropertyType *type : *project()->propertyTypes())
scriptTypes.append(toScriptType(type));
return scriptTypes;
}

QSharedPointer<Document> EditableProject::createDocument()
Expand All @@ -63,6 +73,48 @@ QSharedPointer<Document> EditableProject::createDocument()
return nullptr;
}

ScriptPropertyType *EditableProject::toScriptType(const PropertyType *type) const
{
if (!type)
return nullptr;

switch (type->type) {
case PropertyType::PT_Invalid:
break;
case PropertyType::PT_Class:
return new ScriptClassPropertyType(static_cast<const ClassPropertyType *>(type));
case PropertyType::PT_Enum:
return new ScriptEnumPropertyType(static_cast<const EnumPropertyType *>(type));
}

return nullptr;
}

ScriptPropertyType *EditableProject::findTypeByName(const QString &name)
{
const PropertyType *type = project()->propertyTypes()->findTypeByName(name);
return toScriptType(type);
}

void EditableProject::removeTypeByName(const QString &name)
{
int index = project()->propertyTypes()->findIndexByName(name);
if (index < 0)
return

// TODO the type isn't actually being deleted even when index >= 0
project()->propertyTypes()->removeAt(index);
applyPropertyChanges();
}

void EditableProject::applyPropertyChanges()
{
emit Preferences::instance()->propertyTypesChanged();

Project &project = ProjectManager::instance()->project();
project.save();
}

} // namespace Tiled

#include "moc_editableproject.cpp"
10 changes: 10 additions & 0 deletions src/tiled/editableproject.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include "editableasset.h"
#include "project.h"
#include "scriptpropertytype.h"

#include <QObject>

Expand All @@ -38,6 +39,7 @@ class EditableProject final : public EditableAsset
Q_PROPERTY(QString automappingRulesFile READ automappingRulesFile)
Q_PROPERTY(QString fileName READ fileName)
Q_PROPERTY(QStringList folders READ folders)
Q_PROPERTY(QVector<ScriptPropertyType *> propertyTypes READ propertyTypes)

public:
EditableProject(ProjectDocument *projectDocument, QObject *parent = nullptr);
Expand All @@ -49,10 +51,18 @@ class EditableProject final : public EditableAsset
QString automappingRulesFile() const;
QString fileName() const;
QStringList folders() const;
QVector<ScriptPropertyType*> propertyTypes() const;

Project *project() const;

QSharedPointer<Document> createDocument() override;

Q_INVOKABLE void removeTypeByName(const QString &name);
Q_INVOKABLE ScriptPropertyType *findTypeByName(const QString &name);

private:
ScriptPropertyType *toScriptType(const PropertyType *type) const;
void applyPropertyChanges();
};

inline Project *EditableProject::project() const
Expand Down
2 changes: 2 additions & 0 deletions src/tiled/libtilededitor.qbs
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,8 @@ DynamicLibrary {
"scriptmodule.h",
"scriptprocess.cpp",
"scriptprocess.h",
"scriptpropertytype.cpp",
"scriptpropertytype.h",
"selectionrectangle.cpp",
"selectionrectangle.h",
"selectsametiletool.cpp",
Expand Down
2 changes: 2 additions & 0 deletions src/tiled/scriptmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#include "scriptimage.h"
#include "scriptmodule.h"
#include "scriptprocess.h"
#include "scriptpropertytype.h"
#include "tilecollisiondock.h"
#include "tilelayer.h"
#include "tilelayeredit.h"
Expand Down Expand Up @@ -414,6 +415,7 @@ void ScriptManager::initialize()
registerFileInfo(engine);
registerGeometry(engine);
registerProcess(engine);
registerPropertyTypes(engine);
loadExtensions();
}

Expand Down
40 changes: 40 additions & 0 deletions src/tiled/scriptpropertytype.cpp
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"
131 changes: 131 additions & 0 deletions src/tiled/scriptpropertytype.h
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;
Copy link
Member

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 the EnumPropertyType instance is deleted. There's a few potential solutions we can try:

  • Referring to types using shared pointers, so that the wrapper object can keep the type alive.
  • Adding a pointer to the ScriptEnumPropertyType from the EnumPropertyType, so that it can delete the wrapper upon destruction.
  • Making a copy of the type that is owned by this wrapper. Though that means changes won't automatically apply, but we'd instead need to have a Project.addType or Project.setType to assign a modified object back to the project.
  • Remembering just the name of the type and looking it up each time it is accessed (might get confusing in case the type is renamed).

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.

Copy link
Contributor Author

@dogboydog dogboydog Jun 18, 2024

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

Copy link
Contributor Author

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

};

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; }
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither of those compile right now.

void setColor(QColor value) { mClassType->color = value; }

C:\Users\chris\tiled\src\tiled\scriptpropertytype.h:116: error: C2678: binary '=': no operator found which takes a left-hand operand of type 'const QColor' (or there is no acceptable conversion)

C:\Users\chris\tiled\src\tiled\scriptpropertytype.h(116): note: Conversion loses qualifiers

Or
void setColor(const QColor &value) { mClassType->color = value; }

C:\Users\chris\tiled\src\tiled\scriptpropertytype.h:116: error: C2678: binary '=': no operator found which takes a left-hand operand of type 'const QColor' (or there is no acceptable conversion)
C:\Users\chris\tiled\src\tiled\scriptpropertytype.h(116): note: Conversion loses qualifiers

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's just because mClassType is a const ClassPropertyType*, so you can't modify it. You'd need a non-const ClassPropertyType*.

QVariantMap members() const {return mClassType->members; }
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)

Copy link
Member

@bjorn bjorn Jun 19, 2024

Choose a reason for hiding this comment

The 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 setMembers or addMember, because indeed the members are returned by value here (effectively a copy).

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Member

@bjorn bjorn Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you should get a PropertyValue, so it should at least have exposed its properties like typeName and value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll test again when I get a chance

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it's working for properties that are classes 👍

> tiled.project.findTypeByName('Class12').members['myChild'].value['hello']
$9 = false

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*)