-
-
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 support to Lua exporter for object templates #1901
Conversation
Properties are copied from the template, then merged with the instances's properties before being written to the export.
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.
This is a useful change, though as noted in issue #1850 I think it should be optional.
Unfortunately, no export options exist at the moment so a bunch of code will have to be written to enable this. I imagine it could be done as follows:
- Add this option to the
Preferences
. - Add a nested
struct Options { bool detachTemplates = false };
to theMapFormat
class, and add it as a parameter to thewrite
virtual function (of course, all overrides will need to be adjusted, but at least it won't have to be done again when adding additional options). - When exporting a map, read the option from the preferences and pass it to the
write
function using theOptions
struct. - Pass this option on to the
LuaWriter
and use as appropriate.
src/plugins/lua/luaplugin.cpp
Outdated
writeProperties(writer, mapObject->properties()); | ||
Properties props; | ||
if (const ObjectTemplate *objectTemplate = mapObject->objectTemplate()) { | ||
props.merge(objectTemplate->object()->properties()); |
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.
Here, we could just assign instead of calling merge
, given that props
is still empty anyway.
Please also remove the brackets, since there is only a single statement.
Funny thing, I was a good chunk of the way into doing something close to export options. One of the things I did want to include was CLI overrides for the export command. Spent a chunk of time tossing around what would do the least damage and where to integrate export options. I had not figured out where I wanted to store it, stuffing it into the map file seemed too granular, but sticking it in the global preferences felt kinda buried. A middle ground of a project or a workspace kind of thing feels more like where is should be, but we don't have that yet. Might tackle that as a project later. Also, we get into a bit of a odd spot when an exporter does not react to an export option. Someone will get confused by that. No nice idea comes to mind to solve that. Can't assume that the plugin list is fixed, so I can't just label which options work with what formats. Might just wing it for now and figure it out later. I'll give it some TLC over the next day and work on the export options. |
I find it usually quite annoying that such a large amount of time in programming is spent weighing in questions about what approach to take (or even just how to name something), rather than actually getting something done. But such is the life of a programmer, or maybe mostly the life of a perfectionist programmer... Anyway, if you have other suggestions I'd love to hear them. :-)
Yes, there are already many things that would work better when stored in a project or workspace file. This is covered by issue #1665 and it's on the Roadmap for Tiled 1.3.
Actually all plugins are part of Tiled so we can make sure the setting is recognized in all places where the user would expect it. Of course, there will probably also be a use-case for format-specific options. I have currently no design for this though, especially regarding how the UI would present such options to the user. I imagine a plugin would need to register or expose its options somewhere, such that a UI can be built for it dynamically. But this exporting option is pretty universal, as would be the option to embed tilesets, which can be added in the same way.
Looking forward to it! |
So, I ended up implementing the "Detach Templates" export option independently of the export format (fe13589), so that it is immediately supported by all formats without needing to modify each of them. I think that obsoletes this change, though I'll keep the PR open for now as a reminder that the Lua format should still be fixed to at least refer to the template files if templates are not detached on export. |
* Don't crash when saving a map with a template that failed to load (would have an ObjectTemplate, but without an object). * Only one call to 'merge'.
On second thought I decided to merge this change in addition to the recently added export options, because I realized that in case of the Lua plugin, templates were already written out in a detached form (with all attributes being simply included, regardless of whether the object overrides them), with the exception of the custom properties. I believe this exception was just an oversight. |
Properties are copied from the template, then overwritten by the instances's properties before being written to the export.
Pros:
Cons: