mapeditor / tiled

Flexible level editor
https://www.mapeditor.org/
Other
11k stars 1.74k forks source link

Change tileset definition to avoid renumbering tiles when image width changed #2863

Open bjorn opened 4 years ago

bjorn commented 4 years ago

One problem that's common enough to cause quite some pain is that when a tileset's image width is changed, this causes all the tiles in the tileset to get renumbered. This in turn messes up any tile references from tile layers and tile objects (also affecting object templates) and requires a lot of custom code to fix (a workflow that has always been somewhat broken, see for example issues #1770 and #1851). Even tile references within a tileset definition, like for terrain images, animation frames or Wang sets need adjustment.

I'm considering a solution where adjustment to other assets is entirely avoided, by keeping the tile IDs stable (still pointing to the same location in the image). To achieve this, we can store for each tile its x and y coordinate on the tileset image. Then, when the image has grown in size next time the tileset is loaded, any new tiles will get added to the end of the list, receiving new IDs just like when adding images to an Image Collection tileset.

It would be interesting to store also the width and height of each tile, such that we can at the same time support tilesets that contain tiles of varying sizes (issue #1008), if we add some UI to manage the tiles in a tileset.

One question is what to do when the user changes the parameters of the tileset, like when halving or doubling the tile size. On the one hand, we can't expect this will keep any map using that tileset to still look alright, so it might be OK to just renumber all tiles in this case. On the other hand, this may commonly happen after changing the resolution of the tiles:

Probably, at the last step, Tiled should make an effort to match up tiles based on the old and new size parameters, making sure their IDs stay the same.

bjorn commented 4 years ago

As eishiya pointed out on the forum, in the interest of compatibility it would be a good idea to introduce a new type of tileset for this purpose. This way Tiled can still support the old method where tiles would get renumbered and limit or hide the UI actions, depending on whether the tileset is marked as "spritesheet" or not.

bjorn commented 2 years ago

Suggested format for storing the sub-rectangle for each tile:

For image collection tileset:

<tileset firstgid="1" name="objs" tilewidth="2000" tileheight="2000" tilecount="63" columns="0">
 <grid orientation="orthogonal" width="384" height="332"/>
 <tile id="0" x="0" y="0" width="160" height="192">
  <image width="160" height="192" source="alter.png"/>
 </tile>

For tileset based on tileset image:

<tileset name="Desert" tilewidth="32" tileheight="32" spacing="1" margin="1">
 <image source="tmw_desert_spacing.png" width="265" height="250"/>
 <tile id="0" x="1" y="1" width="32" height="32"/>

Alternatively, a child subrect could be considered, but I think it's not an improvement:

 <tile id="0">
  <subrect x="1" y="1" width="32" height="32"/>
 </tile>

And for JSON:

"tiles":[
 {
  "height": 192,
  "id":0,
  "image":"alter.png",
  "imageheight":192,
  "imagewidth":160,
  "width": 160,
  "x": 0,
  "y": 0
 }, 

Placing this information on the tile object makes sure we can support this feature for both tileset images and image collections.

This approach does exclude image layers, and they already have x, y, width and height attributes, so if this is to be supported for image layers we'd need to use different names there or a child element. Personally I think it would be better to add support for the image layer features to tile objects. That's only locking and image repetition, where the latter should probably be added to tiles.

eishiya commented 2 years ago

I'm in favour of <tile id="0" x="1" y="1" width="32" height="32"/>.

I'm also still against the idea of using Tile Objects as replacements for Image Layers. The subrect child approach or uniquely named properties would be better IMO. It's not consistent with tiles, but Image Layers are not tiles, so I think that's fair. That's not to say Tile Objects shouldn't get these extra properties, I just don't think an overlap in features makes Image Layers obsolete. Image repetition means something different for Tile Objects than it does for Image Layers. On image layers, the idea is infinite repetition, the image is drawn as many times as needed, typically used for background and foreground decoration. Tile Objects have a finite size, repeating an image is useful for e.g. platforms made out of some tile (especially if combined with snap to grid for resizing).

bjorn commented 2 years ago

On image layers, the idea is infinite repetition, the image is drawn as many times as needed, typically used for background and foreground decoration.

This is also one of the problems of image layers in the context of a world though, so I was considering to crop them at the map size. Of course, it may need to be optional in case there is really a use-case for infinite repeating. Alternatively, the option to repeat infinitely could be added to tile objects... ;)

eishiya commented 2 years ago

This is also one of the problems of image layers in the context of a world though, so I was considering to crop them at the map size.

This sounds reasonable, if it's optional. I like to see the infinite repeating on my maps, as it makes it very clear what I'm looking at. I think it would be more intuitive for people new to the feature for the repeating to not be clipped by default, as on smaller maps, the repeat feature may appear to do nothing if clipping is enabled. Unclipped layers are also useful for cases where you have non-repeating but large image layers, such as parallax layers. Perhaps image layers could be clipped only when Show World is enabled and a World is loaded?

Infinitely repeating Tile Objects seem like they would only be useful in scenarios where Image Layers would be a cleaner option xP Though now you've got me thinking it might be cool to be able to assign a tile instead of an image to an Image Layer, so that someone using a background atlas can define all their different backgrounds and then just pick them as needed. Of course, at that point, the argument for using a Tile Object is pretty strong, the only argument against being that you need a layer and an object, and a layer+object for each additional background if they have different parallax values, instead of just an Image Layer, and the associated processing involved with objects in-game (either treating them as dynamic all the time, or taking special care to turn them into static entities at parse-time).

UliAbo commented 2 years ago

That all sounds really like a great feature!

Just from my understanding: When this feature is implemented, I can define (GUI-wise) within a single Tilset multiple Tile sizes? Like based on a "least common divisor" like 16x16px? So I can define Tiles like 16x16 and 32x32 but also non-square ones like 64x32, etc.? That would be really cool as then we could make our game setup much easier, especially when using Tilesets in ObjectLayers.

Did I understand this correctly?

eishiya commented 2 years ago

@UliAbo It's even better than that: you can define tiles of any size, so you can define some tiles that are 16x16, 64x32, etc, but you can also define tiles that are 14x20 or any other size that fits in the image. It would be like having an Image Collection in terms of flexibility, except all the images are arbitrary regions from a single image.

However, you bring up an interesting feature idea: the ability to snap to some grid while defining tiles would be very useful, as many tilesets have tiles designed to be used as multiples of some base tile, and defining larger tiles is just to make them more convenient for e.g. Tile Objects.

Here's a visual example, I used Rectangle Objects on a map to show what tile definitions might look like in this Tile Atlas tileset type: image Most of these are multiples of 16x16px, but the two little trees near the centre are 16x19, and the larger tree above them has two tiles defined that overlap, one of them excludes the transparent padding.

On a related note: this tileset type, along with Image Collections, really calls for per-tile draw offsets xP

Edit: Tileset source & credits: https://www.deviantart.com/chaoticcherrycake/art/Pokemon-Tileset-From-Public-Tiles-358379026

UliAbo commented 2 years ago

@eishiya That would be really wonderful to have! And yes, such a "grid" would be convenient, especially for me also to prevent any mis-definitions of Tiles.

It looks like there's a lot of interest in this feature. Is this already in development, maybe with a rough release schedule in mind?

eishiya commented 2 years ago

No, it's not in development yet AFAIK, since the current focus is on finishing 1.9.

There are also a question about how these tilesets should be displayed:

Oh, another feature idea: Since the main goal of this tileset type is to prevent issues when people change the width of the tileset image, that means it's expected that people will change the size of the tileset image. Tiled needs to make it easy to define the newly added tiles, ideally with some sort of action that defines all the not-yet-defined regions of the tileset based on the tileset's grid settings (which would presumably be set during the tileset creation and optionally used to define the initial tiles).

bjorn commented 2 years ago

No, it's not in development yet AFAIK, since the current focus is on finishing 1.9.

Well, I was recently distracted a little bit and did do some work towards this feature while I helped finish #3339 and then worked on #3359. Though granted, these changes only lay some foundations and no UI work has yet been done, and indeed I think the UI work will stay out of scope for 1.9.

tomcashman commented 3 months ago

Currently encountering this issue on our project in development. From reading the thread, the best "preventative" action is to set our tileset to a large width with a lot of black space for future tiles? This way IDs won't change unless the image width changes?

eishiya commented 3 months ago

Currently encountering this issue on our project in development. From reading the thread, the best "preventative" action is to set our tileset to a large width with a lot of black space for future tiles? This way IDs won't change unless the image width changes?

Extending the tileset's height without changing its width doesn't renumber any tiles, so as long as you only extend your tileset downwards, you shouldn't have any issues, even if your width is narrow. Starting with it wide will give you more free reign to arrange tiles within it. I'd say it's a matter of taste - I personally find it easier to scroll vertically than horizontally, so I prefer to use a very tall but not very wide tileset.

Another remedy is the Mass Replace Tiles script. The easiest way to use this is to create a new tileset any time you want to expand your tileset in a non-compatible way (i.e. wider, or with the tiles arranged in a different way), and then create a mapping from your old tileset to the new one, and use this script to apply that mapping to all the maps in your Project. Using a new tileset makes it easier to create the old-new mapping, since you can see the correct artwork on both the old and new tiles. With this script, you can keep your maps valid while also keeping your tileset compact. Of course, you can use both: you can extend your tileset only downwards while working on it, and then use the Mass Replace Tiles script towards the end of production when your tiles are finished in and you just want to make the tileset more compact or more conveniently laid out.