phaserjs / phaser

Phaser is a fun, free and fast 2D game framework for making HTML5 games for desktop and mobile web browsers, supporting Canvas and WebGL rendering.
https://phaser.io
MIT License
37.22k stars 7.1k forks source link

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

Closed Creepypoke closed 9 months ago

Creepypoke commented 1 year ago

Version

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

bjorn commented 1 year ago

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 commented 9 months ago

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.