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

Export tilesets to Lua #1213

Closed
agersant opened this issue Feb 27, 2016 · 6 comments
Closed

Export tilesets to Lua #1213

agersant opened this issue Feb 27, 2016 · 6 comments
Labels
feature It's a feature, not a bug.

Comments

@agersant
Copy link
Contributor

At the moment, maps can be exported to Lua files but tilesets can not.

As a consequence, the entirety of the tileset data used by a map is embedded in its Lua export. When reusing a tileset across multiple maps, this wastes a lot of space by having each map contain the same tileset data.

A better solution would be to have the exported map include relative paths to exported Lua tilesets. eg:
tilesets = { { firstgid = 1, filename = "../tilesets/town.lua", }, },

This suggested behavior would also make the map export to Lua more consistent with the .json and .js equivalents.

@bjorn bjorn added the feature It's a feature, not a bug. label Feb 27, 2016
@bjorn
Copy link
Member

bjorn commented Feb 27, 2016

Right, at the moment no export-only formats for tilesets are supported, but this would definitely make sense to add. The question is at which point should this file be saved and how should the map realize which file to reference upon export.

@agersant
Copy link
Contributor Author

Thanks for the quick reply! I gave this a lot more thinking and I think I came up with a decent solution which doesn't require drastic code changes.

There are fundamental difference between read-write tileset formats and read-only formats, and also between embedded vs external tilesets. In the current version, these difference are somewhat hidden from the user. This obfuscation creates a lot of confusion (eg. #923, #846) in addition to the limitation which made me open this ticket. Consequently, I think any good solution should involve not only code but also UI changes to make these differences a bit more apparent and let users make informed decisions.

Here are my proposed changes. I tried to preserve the existing workflow as much as possible while limiting the potential for user confusion. I also tried to be forward-compatible with the following hypothetical future features:

Adding tilesets

Currently, the "Map" menu has options for "New tileset" and "Add external tileset" but the Tileset panel only has "New tileset". This is very inconsistent and there's two ways to solve this:

  • Have the two buttons in both locations ("Map" menu and "Tilesets" panel)
  • Have the two buttons in a single location

My recommendation would be to remove the "New tileset" button from the "Tilesets" panel. The reasoning is that all other operations in that list of buttons operate on the current tileset, while adding a tileset really is a map operation.

A great compromise could be to have these options in the "Map" menu and also in the tileset dropdown in the top right of the "Tilesets" panel like this:

Adding tilesets

Tileset operations

Here is what the redesigned button bar in the "Tilesets" panel would look like. The red text is annotations, not part of the suggestion.

Tileset operations

Disembed tileset

  • This is the leftmost button on the row marked "Embedded" in the image above.
  • This button is only visible for embedded tilesets.
  • This opens a "save as"-looking dialog box, offering only the tileset formats Tiled can read from (tsx and json). The tileset is saved to the selected location and the map now treats the tileset as an external tileset. The tileset is no longer editable after pressing this (until embedded again). This is similar to the existing "Export tileset as" button, except it only offers formats which Tiled can read from.

Embed tileset

  • This is the leftmost button on the row marked "External" in the image above.
  • This button is only visible for external tilesets.
  • This switches a tileset from external to embedded, thus making it editable. This is equivalent to the existing "Import tileset" button.

Export tileset

  • This is the second button in the image above.
  • This button is greyed out for external tilesets.
  • This opens a "save-as"-looking dialog box, offering every tileset format Tiled can write to (tsx, json, lua, js, etc.). The tileset is exported to that location. The export-location is saved as a relative path in a new tileset property called "Exports to".
  • When pressing the button again, the tileset's "Exports to" property is used as default location, filename and format in the save dialog.
  • The "Exports to" property can also be edited from the "Tileset Properties" panel.

Another thing to note is how the "Tileset properties" and "Remove tileset" buttons have been moved to the far right of the list, because they're the only ones which are unaffected by the switch between embedded and external.

I got the icons for "Embed" and "Disembed" buttons from the famfamfam set since I could not find a gallery of standard Qt icons. I don't know what the policy is for adding new icons in Tiled.

For "Export tileset", I kept the existing icon in order to minimize unecessary changes, though I think it's hard to tell when this one is greyed out or not.

Exporting a map

  • Embedded tilesets are exported as raw data in the desired format and do not mention any external tileset file.
  • External tilesets which don't have a "Exports to" property are exported as a relative path to the tileset file (tsx or json) referenced by the map.
  • External tilesets which do have a "Exports to" property are exported as a relative path to the exported file (any supported format).

I have been wondering whether saving a map should re-export the tilesets it uses. The tradeoff is that it creates non-intuitive behavior (user presses "Export map" but tilesets are also exported) but it also saves a click on "Export tileset" every time changes are made to the tileset. The full workflow of editing an imported tileset would thus be:

  • Embed
  • Edit the tileset
  • Export tileset
  • Disembed tileset

I think I am in favor of not automating the operation, clarity is more important to me than saving a click here and there. This decision could be relegated to the "Preferences" menu but a decision still needs to be made on the default behavior.

Please let me know what you think, especially if any of this doesn't seem to make sense! Looking into this issue also made me notice UI inconsistency between terrains/animations/collisions editing but I'll save that for another ticket :3

@bjorn
Copy link
Member

bjorn commented Feb 28, 2016

Thanks a lot for all the thought you put into this issue and writing it all down! It really helps with figuring out how to improve this situation.

Actually, making it possible to edit external tilesets without first importing them is currently my number one priority. Unfortunately, I think the problem goes beyond what is reasonably fixable with small UI adjustments in the Tilesets view. This is because when you can edit a tileset as a separate file, it will have its own undo stack as well, which means that either there would be another set of undo/redo buttons in the Tilesets view, or editing the tileset will mean switching to it entirely and adjusting the Tiled interface (like toolbars and available views) accordingly. Of course I'd like to keep the change small, but I think the first option here would be pretty bad. But the latter option is a rather huge change.

About the issue of exporting to read-only formats specifically, the addition of an "Exports to" property makes sense. The only problem there is that there is currently no good way of storing the "format" in which this export should be done. The same issue exists for exporting maps, in which case it would also be nice to store their export location, but it is currently simply not done.

I like the idea of merging the current "import" and "export" actions into an "embed/disembed" toggle. Though I hope we can find icons that are a bit more clear.

I'm not too fond of moving the 'new tileset' and 'add external tileset' actions into the tilesets dropdown, because this would no longer be consistent with the Layers view. Instead, we could just use a menu dropdown in the small tool bar to have both options represented there.

Please let me know also your thoughts on the editing of external tilesets, at the same level as editing of maps. And don't hesitate to open another ticket about the inconsistency between terrains/animations/collisions editing, that you have made me curious about. :-)

@agersant
Copy link
Contributor Author

About the issue of exporting to read-only formats specifically, the addition of an "Exports to" property makes sense. The only problem there is that there is currently no good way of storing the "format" in which this export should be done. The same issue exists for exporting maps, in which case it would also be nice to store their export location, but it is currently simply not done.

Are there a significant technical hurdles here or is it simply a "needs to get done" task? I suppose it will wait until #242 is resolved regardless.

I'm not too fond of moving the 'new tileset' and 'add external tileset' actions into the tilesets dropdown, because this would no longer be consistent with the Layers view. Instead, we could just use a menu dropdown in the small tool bar to have both options represented there.

Great point. I hadn't taken the Layers panel into consideration, and I agree the dropdown would be a good option. It's consistent and wouldn't clutter this button bar further more. I've just noticed that the button for adding layers doesn't have a "Add Layer" tooltip. This one probably would need a "Add Tileset" tooltip.

And don't hesitate to open another ticket about the inconsistency between terrains/animations/collisions editing, that you have made me curious about

Complaint is mostly about how one of them (terrain) can be accessed from the Tilesets panel while the others can't. They also behave fairly differently in that terrain is always on top and keeps focus, while the other two open as regular popups. I remember reading something about conflicting shortcuts between the Terrain window the Tilesets panel and that may be why Terrain behaves like this, but I think they should all be consistent. Might hold off from opening that ticket for now considering the direction #242 is heading.

@bjorn
Copy link
Member

bjorn commented Feb 29, 2016

Are there a significant technical hurdles here or is it simply a "needs to get done" task? I suppose it will wait until #242 is resolved regardless.

It's not a significant technical hurdle. The formats will probably just need some kind of ID (something that is not a translatable string), which you can use to refer to them. Most commonly I guess this will simply be the file extension, except in ambiguous cases.

Complaint is mostly about how one of them (terrain) can be accessed from the Tilesets panel while the others can't. They also behave fairly differently in that terrain is always on top and keeps focus, while the other two open as regular popups.

I have to admit, I no longer remember why the Terrain Editor is modal and the Tile Animation Editor and Tile Collision Editor are non-modal. The Terrain Editor came first, and it probably made something easier, but the latter two depend on interaction with the main window (to change the currently selected tile), so they can't be modal.

radman0x added a commit to radman0x/tiled that referenced this issue Dec 14, 2017
Rough tracer implementation of exporting a tileset directly to a .lua
file. Needs refinement before being finalised.
radman0x added a commit to radman0x/tiled that referenced this issue Dec 14, 2017
Working but code is VERY rough, needs tidy.

* LuaPlugin now uses Tiled::Plugin
* FileFormat put in it's own file fileformat.h
* FormatHelper now works for Tilesetformat and MapFormat
radman0x added a commit to radman0x/tiled that referenced this issue Dec 19, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 19, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 19, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 19, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
radman0x added a commit to radman0x/tiled that referenced this issue Dec 20, 2017
* Rename of standalone to embedded for semantic correctness
* Moved writeColor to a static member
* Misc code style adjustments
@bjorn bjorn closed this as completed in 33c3b8b Dec 20, 2017
@bjorn
Copy link
Member

bjorn commented Dec 20, 2017

A better solution would be to have the exported map include relative paths to exported Lua tilesets. eg:
tilesets = { { firstgid = 1, filename = "../tilesets/town.lua", }, },

Hmm, this issue is now closed but that feature would still need to be implemented somehow. As a workaround you would have to replace the file extension, assuming you did a Lua export with the same name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature It's a feature, not a bug.
Projects
None yet
Development

No branches or pull requests

2 participants