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.08k stars 7.09k forks source link

Collision bug on specific tilesets when using setTexture, setFrame, setSizeToFrame #3861

Closed tarsupin closed 6 years ago

tarsupin commented 6 years ago

It's a little hard to explain this bug, and it took me a long time to even figure out what's happening. Unfortunately, even now that I understand the problem, I can't figure out why the code isn't working.

This bug appears to apply if the texture is changed during a scene rather than pre-loaded. (At least, as far as I know. I haven't encountered a problem with other collisions thus far to my knowledge.).

For example, in my case, I have a custom bullet class that extends Phaser.Physics.Arcade.Image. And when a bullet is loaded using .get, it will also update the texture to match whichever bullet is being fired at that time. The actual texture changes fine. However, the collision detection doesn't update properly.

This bug is very noticeable in tilesets where the first frame is of different sizes than the other frames. See the following image:

particles

In this example, the first frame is quite large (128x128). When a tiny laser is fired and collides with a target, it is uses the 128x128 frame for its collision.

Note that if I change the positions of the "frames" array so that "ShipTrailBlue" is not in the first position in particles.json, everything works fine. See below:

{
    "filename": "LaserBlueLong",
    "rotated": false,
    "trimmed": false,
    "sourceSize": {
        "w": 30,
        "h": 8
    },
    "spriteSourceSize": {
        "x": 0,
        "y": 0,
        "w": 30,
        "h": 8
    },
    "frame": {
        "x": 1,
        "y": 131,
        "w": 30,
        "h": 8
    }
},
"frames": [
{
    "filename": "ShipTrailBlue",
    "rotated": false,
    "trimmed": false,
    "sourceSize": {
        "w": 128,
        "h": 128
    },
    "spriteSourceSize": {
        "x": 0,
        "y": 0,
        "w": 128,
        "h": 128
    },
    "frame": {
        "x": 1,
        "y": 1,
        "w": 128,
        "h": 128
    }
},

In this case, the collision matches the "LaserBlueLong" entry instead, which is much closer to the size of the other bullets and lasers.

Going back to the 128x12x8 frame being in the first slot (where the issue is more noticeable), here's the code where it became problematic. Within the custom phaser bullet class, I have the following:

if(bulletType) { this.setFrame(bulletType); }
if(bulletType) { this.setTexture("particles", bulletType); }
this.setSizeToFrame(bulletType);

// If this is set, it reduces the size, but the offset position is still linked to the first frame, which makes for a very bizarre collision in the bottom-right corner of things.
this.setSize(10, 10);

All of the frame updates change the appearance of the image correctly, and also appear to update the data values correctly. So I'm not sure why (or where) the code begins to care about the first frame.

photonstorm commented 6 years ago

Some pointers:

1) Calling setTexture automatically calls setFrame internally.

2) Calling setFrame (either directly or via setTexture) automatically calls setSizeToFrame, as long as the Game Object has the size component, which Image does.

3) Doing either of these has absolutely no impact on any physics bodies. You talk about collision and tilesets in the issue title, but there's no code or examples of them in the actual issue, so I'm just guessing here, but I can say for sure that changing the frame of an Image doesn't adjust its Body size. Where Tilesets come into this I'm not sure tbh.

tarsupin commented 6 years ago

1 and 2. Yes, I'm aware. Since there's no conflict of reusing them, I was showing each of them to indicate that each one of them was being vetted.

  1. If changing the texture/frame doesn't affect physics bodies and collisions, then shouldn't the methods describe updating the size of the Game Object be consistent in that? The setSize method says "Sets the size of this Game Object" as it's only purpose, and it does affect the collision. The "size of Game Object" is repeated in both the setSizeToFrame and setFrame as a default parameter.
photonstorm commented 6 years ago

I don't really get what you mean, sorry. They say they set the size of the Game Object, which they do. Nowhere do they say they'll change the size of a physics body, because they don't. The size is used when a body is created of course but it doesn't then change beyond that (well, unless using AP and syncBounds is set to true but that's quite rare I think)

Probably easier to explain your problem with some code.

tarsupin commented 6 years ago

Well, the issue was raised because of my incorrect assumption that setTexture would update the physics body, which I suppose renders the code and indeed the entire issue irrelevant.

But I am confused if there is any path that exists to update the physics body when a texture changes, because I've tried using body.updateBounds() after updating the frame, and that doesn't appear to have the desired effect either.

Is there a sequence / method that does properly update the bounds of the physics body to the existing frame? Perhaps it's my unfamiliarity with the code, but I can't find one in the API. updateBounds() looks like it would be the only relevant method. And if a solution doesn't exist, I'd like to change this issue into a feature request, because that's definitely a feature I would need (and would assume many others would as well?).

photonstorm commented 6 years ago

It could be done, but you'd have to instigate it from the Body itself, because the Game Object has no concept of having a body or not, so it can't happen directly from setFrame. Also, there are plenty of reasons you wouldn't want it to happen automatically - for example, playing an animation shouldn't cause the body to resize every frame.

Body has a setSize method already. The easiest approach by far, I think, would be to make it so that if you don't provide a w/h to that method, it uses the frame size. So you'd change the frame, then call that and it'd use the new frame size.

photonstorm commented 6 years ago

Thanks for opening this issue, and for submitting a PR to fix it. We have merged your PR into the master branch and attributed the work to you in the Change Log. If you need to tweak the code for whatever reason please submit a new PR.