mapeditor / tiled

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

Scripting: layer instanceof TileLayer produces a type error #3232

Open yanchith opened 2 years ago

yanchith commented 2 years ago

Hey :)

Thanks for making Tiled ❤️

When implementing an exporter to a custom level format from Tiled's TileMap, I managed to produce a type error (TypeError: Type error) from the script when doing:

const isTileLayer = layer instanceof TileLayer; // For the record, layer really was a TileLayer

In the d.ts, TileLayer is defined as:

declare class TileLayer extends Layer

.. but I imagine this is probably not a JS class, but Tiled's data structure that is just accessible from JS, but it likely doesn't implement all the operators.

I know, there's Layer.isTileLayer, but this probably shouldn't crash.

Also, I checked other layer types (GroupLayer, ImageLayer and ObjectGroup), and they behave the same. Not sure about other API types.

Platform: Windows Tiled Version: 1.7.2

bjorn commented 2 years ago

.. but I imagine this is probably not a JS class, but Tiled's data structure that is just accessible from JS, but it likely doesn't implement all the operators.

You're right, the layer is not actually an instance of TileLayer. It is a JS object wrapping an instance of the C++ class Tiled::EditableTileLayer. And the TileLayer variable is not actually a class either. It is a JS object wrapping the "metaobject" of that C++ class, which allows calling the constructor and accessing enums (though this class doesn't have any, but for example like TileMap.Orthogonal).

So, this whole setup has very little to do with how things normally work in JavaScript, and I think instanceof raising a type error is probably the correct thing to do in this case. I've marked this as a Qt issue, but I'm not sure if it can be fixed. Are you saying it may be possible to make this work by implementing some operators?

Regarding the TypeScript definitions, would it help if they were defined using interfaces instead of class definitions, or do interfaces raise the same expectations regarding instanceof?

bjorn commented 2 years ago

I found QTBUG-79868 regarding the throwing of TypeError, which was apparently fixed to return false instead. Could be interesting to look into why we're still getting a TypeError in this case. This function might be related.

yanchith commented 2 years ago

So, this whole setup has very little to do with how things normally work in JavaScript, and I think instanceof raising a type error is probably the correct thing to do in this case. I've marked this as a Qt issue, but I'm not sure if it can be fixed. Are you saying it may be possible to make this work by implementing some operators?

Sorry, I don't actually have real experience with integrating with Qt's javascript VM, I just assumed there might be something like that, and that function you mentioned seems to be the place to look.

Regarding the TypeScript definitions, would it help if they were defined using interfaces instead of class definitions, or do interfaces raise the same expectations regarding instanceof?

Since interfaces do not actually exist in JS, there should be no expectations of instanceof reporting anything useful. It still probably shouldn't crash, but less people would try :)