Give scripts access to existing file formats #2716
Conversation
|
@bjorn Any idea why returning the custom types doesn't work? I did the I think there's an extra step or condition I'm missing. I just couldn't manage to find it. The code that throws the error eventually checks QPropertyCache, and that was difficult to trace back farther, so I can't tell where along the way it comes up with QMetaType::Undefined for the Q_INVOKABLE's return type. |
The other thing that frequently happens to me as well, is that the type is not recognized because it is in a namespace. That is because if you use Q_INVOKABLE or if you use a type in a Q_PROPERTY, the returned type is encoded by string somewhere and later looked up again. In that case, you need to use "Tiled::EditableSomething" for example instead of just "EditableSomething" even if the compiler will understand the type either way. |
|
Thanks, that fixed it! I'll remember that for the future. |
|
This is now blocked on #2720 , to avoid some messy merge conflicts in the scripting API reference. |
There was a problem hiding this comment.
Nice addition! I've left a number of inline comments.
It's too bad we need those wrappers even though the original objects derive from QObject, which as far as I understand is simply because we need to work with editable pointers rather than "raw" maps and tilesets. One day I hope to find a way to unify this, but for now the wrapper is a good solution (besides, it's not the only difference, there's also the different ways of reporting errors...).
Scripts can now call pre-defined map/tileset formats, and can query what map/tileset formats are available. Closes mapeditor#2696. Still need to write documentation for the new APIs. I wasn't able to get QJSEngine to accept actual ScriptTilesetFormatWrapper return values, so functions that return those use QObject instead. Same applies to ScriptMapFormatWrapper.
Qt requires a namespace to resove a class name into a metatype correctly. It seems to do a string comparison at some point in its code against the actual symbol written as the return type for a Q_INVOKABLE function, ignoring how that symbol resoves.
Also fixed a bug making `read` fail (unrecognized return type).
Phlosioneer
force-pushed
the
2696-access-other-file-loaders
branch
from
0ea6dee to
ce63b59
Compare
|
The MapFormat/TilesetFormat interface already throws the error if control reaches a non-overrided function. However, thinking about it, that may not be an actual javascript error; it might just be a message in the console? I'll check. |
|
Github won't let me respond to the last two messages you left, so I'll reply here:
In that case I would rather not expose the nameFilter at all, then. It's not useful for scripts, we already have the supportsFile function. |
Removed `name` and `extension` properties. Added checks for whether a file format is able to read/write.
There are really only two built-in formats, which are the 'tmx' and 'tsx' formats. All the others are added by plugins which can be disabled by the user. Regardless, I don't see much value in documenting this list of formats and then having to maintain those lists (which will surely be forgotten).
|
Nice work, thanks! |
|
Uh, your commits didn't pass the compile tests. Though it seems to be a CI error. |
|
@Phlosioneer Heh, they failed because the pull request was already closed when they started. |
Scripts can now call pre-defined map/tileset formats, and can query what
map/tileset formats are available. Closes #2696.
Still need to write documentation for the new APIs.
I wasn't able to get QJSEngine to accept actual ScriptTilesetFormatWrapper
return values, so functions that return those use QObject instead. Same
applies to ScriptMapFormatWrapper.
Suggestions of better names for ScriptTilesetFormatWrapper/ScriptMapFormatWrapper are welcome.