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

Incorrect type for Tiled custom properties field in Tilemap and another classes #6331

Closed
Creepypoke opened this issue Dec 26, 2022 · 5 comments

Comments

@Creepypoke
Copy link

Version

  • Phaser Version: v.3.55.2
  • Operating system: Windows 11
  • Browser: All browsers

Description

Tilemap and other entities are working with Tiled map data has incorrect type annotation for properties field.

https://github.com/photonstorm/phaser/blob/ec18bf6ac77f1ae6e36258d1105b831069984c28/src/tilemaps/Tilemap.js#L191-L198

On Tiled documentation pages the scheme of Map properties field is described as array of properties, not an object. This is correct for another Tiled entities, like layer, chunk, objects, etc.

Example Test Code

I prepared a little example with simple JSON map exported from Tiled 1.9.2.
https://codesandbox.io/s/cool-surf-vbetsr?file=/scenes/HelloWorldScene.ts

In example we can see that's array's methods don't be allow for tileset.properties field.

Additional Information

I made some jsdoc types for describe Tiled custom properties and check how it generate in tsgen utility in Phaserjs repository. It generate correct TS types, but I don't so confident for edit existing Tilemap and another classes. if it is useful please feel free to use it.

@photonstorm
Copy link
Collaborator

Pretty sure this is a recent change in Tiled itself. It didn't use to be an array, so the correct type, to support multiple versions of Tiled, would need to be (object|object[]).

@Creepypoke
Copy link
Author

Creepypoke commented Dec 29, 2022

Yes, you right. I asked in Tiled discord about format and how long it was changed.

TL;DR

The object format of properties is outdated since 2018 and Tiled team haven't a plan to support it.

From discord discussion:

In Tiled 1.1-ish, the JSON format was different and was in fact a dictionary, with the types being stored in a separate dictionary.

This is what custom properties used to look like in Tiled 1.1 JSON (you can still export this from Tiled using the json1 plugin):

"properties":
{
   "test":"",
   "test2":""
},
"propertytypes":
{
   "test":"string",
   "test2":"string"
},

But starting with Tiled 1.2, the JSON format uses an array.
Here's the relevant changelog: https://doc.mapeditor.org/en/stable/reference/json-map-format/#tiled-1-2


How do you think, is it necessary to support outdated format for compatibility?

@photonstorm
Copy link
Collaborator

Yes we need both formats. Tiled is the kind of app that devs don't often update.

@bjorn
Copy link

bjorn commented Jan 6, 2023

Indeed the JSON format changed in Tiled 1.2, which I don't expect anybody to use anymore. However, Tiled retains the ability to save maps in the old format even in the latest version, by disabling the "json" plugin and enabling the "json1" plugin instead in Preferences > Plugins. If Phaser has not yet been updated to support the new format, I expect Tiled users have been switching to the "json1" plugin.

It would in any case be good to support both formats, to make life easier for both new and existing users. :-)

@photonstorm
Copy link
Collaborator

Thank you for submitting this issue. We have fixed this and the fix has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants