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

PutTileAt sid wrong if layer does not use all tilesets #5931

Closed
christianvoigt opened this issue Nov 30, 2021 · 11 comments
Closed

PutTileAt sid wrong if layer does not use all tilesets #5931

christianvoigt opened this issue Nov 30, 2021 · 11 comments

Comments

@christianvoigt
Copy link

Version

  • Phaser Version: v3.60.0 Beta 3
  • Operating system: Windows 10
  • Browser: Chrome

Description

I am using a tilemap JSON file from Tiled with three embedded tilesets ("tileset1", "tileset2", "tileset3"). I have defined a new tile layer in Tiled "MyNewLayer" and recreated it in my game with:

map.createLayer(
      "MyNewLayer",
      [tileset3],
      0,
      0
    );

If I use map.putTileAt(tileGid, x, y, false, myNewLayer) the following error is thrown:

TypeError: Cannot read properties of undefined (reading 'tileWidth')

The issue is caused by var sid = tiles[index][2]; in PutTileAt.js. This tileset id is wrong if not all tilesets have been added to the layer. In my case Phaser assumes the sid is "2" when it should actually be "0".

So if I change my code accordingly, the error disappears:

map.createLayer(
      "MyNewLayer",
      [tileset1, tileset2, tileset3],
      0,
      0
    );

It is probably not the intention to require that all tilesets have to be added to all layers, so I think this is a bug.

@samme
Copy link
Contributor

samme commented Dec 1, 2021

What is your putTileAt() code exactly?

@christianvoigt
Copy link
Author

Hi @samme,
I have now created a repository with a minimal example: https://github.com/christianvoigt/phaser-issue-5931

The example is using a tiled tilemap from this tutorial. I have simply added two additional tilesets and an additional empty layer to the tiled file.

After installing all packages with npm install, use npm run watch to start the example. If you open the console in the project's browser window, you should see the error Uncaught TypeError: Cannot read properties of undefined (reading 'tileWidth').

Here is the code of the example:

import "phaser";

export default class Demo extends Phaser.Scene {
  constructor() {
    super("demo");
  }

  preload() {
    this.load.image("tiles2", "assets/tilesets/debug.png");
    this.load.tilemapTiledJSON("map", "assets/tilemaps/tuxemon-town.json");
  }

  create() {
    const map = this.make.tilemap({ key: "map" });
    const tileset2 = map.addTilesetImage("debug", "tiles2");
    const myNewLayer = map.createLayer("MyNewLayer", [tileset2], 0, 0);
    const tileGid = tileset2.firstgid;
    map.putTileAt(tileGid, 5, 5, false, myNewLayer);
  }
}

const config = {
  type: Phaser.AUTO,
  backgroundColor: "#125555",
  width: 800,
  height: 600,
  scene: Demo,
};

const game = new Phaser.Game(config);

@fielding
Copy link

I ran in to this as well and meant to open a PR, but instead just made a TODO in my code and as a workaround added each tileset to every layer so that sid are correct.

@christianvoigt I'll probably still open a quick PR for it if you aren't already

@christianvoigt
Copy link
Author

@fielding great, go ahead, I'm currently not working on the project where I use this.

@Arcanorum
Copy link

I can't seem to get this working even with adding each tileset to every tilemap layer. :/

@OdinvonDoom
Copy link

OdinvonDoom commented Jun 26, 2023

Any progress here? The same issue comes up if you create multiple tilesets and multiple layers. One of most frustrating Phaser tasks yet have been creating a tilemap NOT using Tiled...

the GID system is very unclear. It seems as if every added tileset stacks it's tiles/frames on top of the other, so if you add 3 tilesets of 50 frames each, tileset 1 will use frames 0-49, 2 will use 50-99, and 3 will use 100-149. I had this working briefly, just having to tell each tileset with it's first gid is:
const tileset3 = map.addTilesetImage('imgTileset3', undefined, undefined, undefined, 0, 0, 100);
then using it in the layer:

const layer3 = map.createBlankLayer("Layer3", tileset3);
layer3 ?.putTilesAt(2Darray-of-Indecies-100-149], 0, 0, false);

I recently had to introduce a 4th layer before the other 3, so I adjusted all the numbers, but no dice, constant error at the same place, when putTileAt() is used internally from calling putTilesAt().

I can't tell what is even the point of telling each layer which tileset(s) it should use.

@samme
Copy link
Contributor

samme commented Jun 26, 2023

This may be #6474 .

GIDs are "global" because they're unique within the tilemap (not a tileset). You assign GIDs with the gid argument in addTilesetImage(). You can make GIDs contiguous or not; as long as they don't overlap it's fine.

@OdinvonDoom
Copy link

Yah I noted all of that, and it more-or-less would be fine on it's own, but causes this error when a particular layer is assigned not-all tilesets, hence having to assign all tilesets to all layers, making any assignment effectively meaningless.

I noticed while debugging with 4 tilesets and 4 layers, that if I assigned tilesets indexed 1 & 2 to layer 0,
map.createBlankLayer("Layer0", [tileset1, tileset2]);
and if I tried to putTile() from tileset 2, it would retrieve the "global" sid of 2, but try to use that to reference the "local" array that was passed in, now having indices 0 and 1. Thus, the error.

So it seems that you need to include at least every tileset from the start:
map.createBlankLayer("Layer anything up to index 2", [tileset0, tileset1, tileset2]);
map.createBlankLayer("Layer anything up to index 3", [tileset0, tileset1, tileset2, tileset3]);
etc.

@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.

@poodoff
Copy link

poodoff commented Jul 31, 2023

@photonstorm
Hi.
I try this example https://labs.phaser.io/edit.html?src=src\tilemap\base%20tile%20size.js ( \tilemap\base tile size)
In console log I see: Phaser v3.61.0-beta.1

In example I tried to use this code
layer.putTileAt(1,2,2); ( just for example )

After this line 26:
layer.randomize(0, 0, map.width, map.height, [ 0, 1, 2, 3, 4, 5, 6, 7 ]);

But in console I got error message:

Uncaught TypeError: layer.tilemaplayer is undefined
    PutTileAt https://labs.phaser.io/build/3.61.0-beta.1.js:223626
    putTileAt https://labs.phaser.io/build/3.61.0-beta.1.js:219734
    create https://labs.phaser.io/frame.html line 75 > eval:28
    create https://labs.phaser.io/build/3.61.0-beta.1.js:193457
    loadComplete https://labs.phaser.io/build/3.61.0-beta.1.js:193364
    emit https://labs.phaser.io/build/3.61.0-beta.1.js:200
    loadComplete https://labs.phaser.io/build/3.61.0-beta.1.js:118333
    fileProcessComplete https://labs.phaser.io/build/3.61.0-beta.1.js:118299
    onProcessComplete https://labs.phaser.io/build/3.61.0-beta.1.js:117098
    onload https://labs.phaser.io/build/3.61.0-beta.1.js:123375 

@photonstorm
Copy link
Collaborator

@poodoff yeah someone told me on Discord, this is now fixed in main branch and will be in 3.61 Beta 2.

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

7 participants