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
36.94k stars 7.08k forks source link

TS Type wrong: Children of Phaser.Physics.Arcade.StaticGroup should not be Phaser.GameObjects.GameObject #6402

Closed DerZyklop closed 6 months ago

DerZyklop commented 1 year ago

Version

Description

I wanted to rewrite my JS project to TypeScript. It's running fine, but I have some Type errors from types that come from phaser’s return types. Here is one:

const someBalls = this.physics.add.staticGroup(); // Type of someBalls is Phaser.Physics.Arcade.StaticGroup now
someBalls.create(100, 200, "gems", Phaser.Math.Between(0, 4));
const firstBall = someBalls.getChildren()[0];
firstBall.setCircle(20); // ERROR: Property 'setCircle' does not exist on type 'GameObject'.

I debugged, and the type of firstBall shows as ArcadeSprite2 in the browser. I tried to find a type that seems close to that and found Phaser.Physics.Arcade.Sprite.

So i can workaround with as like this:

const someBalls = this.physics.add.staticGroup();
someBalls.create(100, 200, "gems", Phaser.Math.Between(0, 4));
const firstBall = someBalls.getChildren()[0] as Phaser.Physics.Arcade.Sprite;
firstBall.setCircle(20);

Example Test Code

See the example above.

Additional Information

Phaser.Physics.Arcade.StaticGroup extends the class Group, which defines children.

        class Group extends Phaser.Events.EventEmitter {
            …

            /**
             * Members of this group.
             */
            children: Phaser.Structs.Set<Phaser.GameObjects.GameObject>;

I don’t know much about phaser yet but that is clearly a wrong type. I assume the children of Phaser.Physics.Arcade.Sprite are always "ArcadeSprite’s", or if it’s related to what create(…) does internaly, but if create(…) has an effect on the childrens type, then Phaser.Physics.Arcade.StaticGroup should be generic like

        class Group<ChildrenType extends Phaser.GameObjects.GameObject = Phaser.GameObjects.GameObject> extends Phaser.Events.EventEmitter {
            …

            /**
             * Members of this group.
             */
            children: Phaser.Structs.Set<ChildrenType>;

So that 'Phaser.Physics.Arcade.StaticGroupcan extendGroup` like:

    class StaticGroup extends Phaser.GameObjects.Group<Phaser.Physics.Arcade.Sprite> {
    …
DerZyklop commented 1 year ago

Update: My improved workaround…

Instead of staticGroup()’s Phaser.Physics.Arcade.StaticGroup, i use my own AcardeStaticGroup. In the definition, I overwrite the children's type:

export type AcardeStaticGroup = Omit<Phaser.Physics.Arcade.StaticGroup, 'getChildren' | 'children'> & {
    children : Phaser.Structs.Set<Phaser.Physics.Arcade.Sprite>;
    getChildren() : Phaser.Physics.Arcade.Sprite[];
}

Then i can use it like this:

const someBalls = this.physics.add.staticGroup() as AcardeStaticGroup;
someBalls.create(100, 200, "gems", Phaser.Math.Between(0, 4));
const firstBall = someBalls.getChildren()[0];
firstBall.setCircle(20);

But there would be more work to do if I follow this approach.

DerZyklop commented 1 year ago

I just found another flaw in the same class. Group has .create(…) which returns any. In fact it returns the exact same thing that .children is an set of.

.children currently has the type children: Phaser.Structs.Set<Phaser.GameObjects.GameObject>; Thus .create(…) should probably return Phaser.GameObjects.GameObject.

Or better following my abstract proposal from above: If .children is children: Phaser.Structs.Set<ChildrenType>; then .create(…) returns ChildrenType.

DerZyklop commented 1 year ago

I see two possible solutions here:

Option 1

class Group<ChildType extends Phaser.GameObjects.GameObject> …

So that every code that uses the type Group is forced to deliver a ChildType like:

class FancyThingsGroup extends Group<FancyThing> …

Option 2

class Group<ChildType extends Phaser.GameObjects.GameObject = Phaser.GameObjects.GameObject> …

So that all existing classes that use it, can be unchanged.

Pro for Option 2

Contra for Option 2

Both options would allow changing the class StaticGroup to use extends Phaser.GameObjects.Group<Phaser.Physics.Arcade.Sprite> (my initial issue).

musjj commented 11 months ago

Classes extending Group without overriding the return types of the methods is why issues like this happen. Another one:

const group = this.physics.add.staticGroup()
const sprite = group.create(...) // `any`, preferably it should be `Phaser.Types.Physics.Arcade.SpriteWithStaticBody`

https://github.com/photonstorm/phaser/blob/cbb802bcff470e3dc222986f5b71126d6741307c/src/gameobjects/group/Group.js#L287-L290

I'm not really familiar with tsgen, but is there a way to override the types of the class being extended?

A workaround that works for me is to manually create the object and add it to the group:

const group = this.physics.add.staticGroup([
  this.physics.add.staticSprite(100, 100, "key").setScale(2),
]);

// or

group.add(this.physics.add.staticSprite(100, 100, "key"));
photonstorm commented 6 months ago

There is absolutely no easy way to do this from the JSDocs and tsgen alone. The code is valid, there are TS sanity work-arounds and that's the only realistic way to achieve this for now, beyond a complete rewrite of Phaser in TS.