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

Logic for TilemapLayer.skipCull seems to be backwards. #5524

Closed
veleek opened this issue Jan 24, 2021 · 4 comments
Closed

Logic for TilemapLayer.skipCull seems to be backwards. #5524

veleek opened this issue Jan 24, 2021 · 4 comments

Comments

@veleek
Copy link
Contributor

veleek commented Jan 24, 2021

Version

  • Phaser Version: Phaser v3.52.0 (WebGL | Web Audio)
  • Operating system: Windows

Description

I have an issue where tiles in a TilemapLayer are culled unexpectedly. In order to verify this I want to ignore all culling for the TilemapLayer (e.g. render all tiles). I would expect that setting skipCull = true would result in no culling occurring so all tiles will be rendered.

For consistency with the culling done by the camera system (see logic in BaseCamera here) the Tilemap cull functions like IsometricCullTiles should return all tiles when skipCull is true, but they currently return no tiles.

Tiles being culled is a separate issue related to putting a TileMapLayer in a container. See #5494 (comment) for a bit more detail on that problem. I believe that's unrelated to this specific skipCull issue.

Example Test Code

    this.map = scene.make.tilemap({ key: name });

    this.tilesetName = tilesetName;
    const tileset = this.map.addTilesetImage(tilesetName);

    this.floorLayer = this.map.createLayer("floor", tileset).setSkipCull(true);

No tiles are rendered. Change to setSkipCull(false) renders tiles (and culls some of them).

Additional Information

Rename skipCull to disableCull. Update cull function in TilemapLayer to return all tiles if disableCull is true.

@veleek
Copy link
Contributor Author

veleek commented Jan 24, 2021

Tried out an implementation of TileMapLayer.cull that looks like this:

cull: function (camera)
    {
        if(this.skipCull)
        {
            return this.layer.data.flat();
        }

        return this.cullCallback(this.layer, camera, this.culledTiles, this._renderOrder);
    }

And it seems to work. This is using the ES10 Array.flat() method. But a more widely available option is also fine. I just picked the shortest option from this list: https://www.jstips.co/en/javascript/flattening-multidimensional-arrays-in-javascript/. If you think is is reasonable let me know and I'll create a PR with the change (along with removing the skipCull logic from the individual cull methods, and renaming the property.

@veleek
Copy link
Contributor Author

veleek commented Feb 8, 2021

@gammafp - Are there any guidelines or requirements around which JS APIs we need to build phaser against? If I get an answer to this then I can go ahead and submit a PR.

@samme
Copy link
Contributor

samme commented Sep 8, 2021

I think IE 10 is the minimum.

@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

4 participants