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

Auto-adjusting tile references when a tileset has grown doesn't update tile types and properties #3236

Closed
albertvaka opened this issue Jan 15, 2022 · 4 comments · Fixed by #3237
Assignees
Labels
bug Broken behavior.

Comments

@albertvaka
Copy link
Contributor

Bug description

When Tiled detects that a tileset image has changed size, it offers the option to update all the references to it to reflect the new tile IDs.

However, if tiles in the spritesheet had types and/or properties defined, they remain assigned to the old tile IDs they were assigned to previously, which after the re-enumeration is now incorrect.

Steps to reproduce:

  1. Create a small tileset with 2x2 tiles:
    spritesheet

  2. Add a type to the first tile of the second row (id=2).
    Screenshot 2022-01-15 01 43 36

  3. Resize the tileset image to be 4x2 tiles:
    spritesheet

  4. When prompted in Tiled, accept to auto-update the tile references.
    image

  5. Check that the tile you previously given the property is now tile id=4, but the type is still assigned to tile id=2.
    image

@eishiya
Copy link
Contributor

eishiya commented Jan 15, 2022

Nitpick: The wording of the prompt is correct, it fixes references. Properties and types assigned to tiles are not references xP

I think this should probably have its own prompt that would show up if any tiles had properties or types set, since it's a separate thing from updating maps, and since sometimes it's more desirable to delete the existing properties and clear the types, such as when the number of columns changes because the tile size is changed, rather than because the image changed, or when the image is replaced entirely.

Two prompts in a row would be annoying, but I remember there being talk of removing the tile reference prompt in favour of a menu item to reassign map IDs based on old/new tileset sizes, so that users can run it at their convenience and rather than being given one chance (#2866), potentially messing up any maps that weren't open at the time. There's also been talk of making a new spritesheet-like tileset type as the default, which would maintain tile IDs even when the image is resized, which would make these reference fixes unnecessary (#2863).

Related issue: the same issue with Terrains, includes some discussion of potential problems with this sort of automatic arrangement: #1851

@albertvaka
Copy link
Contributor Author

Hi! Thanks for reviewing this 😄 Two prompts might be annoying indeed, but we could reword the current one and add checkboxes for the different updates that can be performed at once. That seems way less work than the editor to assign arbitrary IDs, so even if that's going to happen long-term we might want to start with the prompt changes. Wdyt?

@bjorn
Copy link
Member

bjorn commented Jan 16, 2022

However, if tiles in the spritesheet had types and/or properties defined, they remain assigned to the old tile IDs they were assigned to previously, which after the re-enumeration is now incorrect.

Hmm, I see I forgot the type, but the properties should get adjusted. The related code is at https://github.com/mapeditor/tiled/blob/master/src/tiled/adjusttileindexes.cpp#L176.

And yeah, actually it adjusts references and tileset data, but the whole approach is not ideal.

@bjorn bjorn added the bug Broken behavior. label Jan 16, 2022
@bjorn bjorn added this to Tiled 1.8 Jan 16, 2022
@albertvaka
Copy link
Contributor Author

albertvaka commented Jan 16, 2022

You are totally right: properties, probabilities and the other tile fields are already updated. That means most of the code needed is already there 😃 I will try to open a PR that adds the missing type update to the existing code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken behavior.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants