Bug when no frameMax parameter is specified to Loader.spritesheet

[Lucas-C]

This bug occurs only from the 1680th frame in the spritesheet

This is a bug in the API:

• Phaser version(s): 2.6.2
• Live example: https://chezsoi.org/lucas/phaserjs_spritesheet_bug_repro.html?1767
• What should happen: without the ?1767 query string fixing the frameMax parameter to Loader.spritesheet, the same sprite should be displayed
• What happens instead: when no frameMax is passed to Loader.spritesheet (default value is -1), the first sprite of the spritesheet is displayed instead. But all is fine if we try to render the sprite before (1679th one)

There is the code used in the live example:

window.onload = function () {
  var game = new Phaser.Game(800, 600, /*renderer=*/Phaser.AUTO, /*parent=*/'',
                             /*state=*/{preload: preload, create: create},
                             /*transparent=*/false, /*antialias=*/false)

  function preload() {
    // Test with ?1767 query string
    var frameMax = window.location.search ? +window.location.search.substring(1) : -1
    game.load.spritesheet('MySpritesheet', './roguelikeSheet_transparent.png', 16, 16,
                          /*frameMax=*/frameMax, /*margin=*/0, /*space=*/1)
  }

  function create() {
    // The following will render the first tile if loaded with frameMax=-1, whereas 1679 is OK
    var sprite = game.add.sprite(game.world.centerX, game.world.centerY, 'MySpritesheet', 1680)
    sprite.anchor.setTo(0.5)
    sprite.scale.setTo(4)
  }
}

The API function called is Loader.spritesheet, but I think the issue relies in AnimationParser.spriteSheet

[Lucas-C]

The spritesheet loaded has 57x31 square sprites of size 16 pixels.
However AnimationParser.spriteSheet code only counts 56 rows x 30 columns (hence the 1680 number)

I think the code should be:

var row = Math.floor((width - margin + spacing) / (frameWidth + spacing));
var column = Math.floor((height - margin + spacing) / (frameHeight + spacing));

[samme]

game.load.spritesheet('MySpritesheet', './roguelikeSheet_transparent.png', 16, 16, /frameMax=/frameMax, /margin=/0, /space=/1)

Does this work as expected if you use margin = -1, spacing = 1?

[Lucas-C]

Yes, this would give the correct result result mathematically:

var row = Math.floor((width - margin) / (frameWidth + spacing))
        = Math.floor((985 - (-1)) / (16 + 1))
        = Math.floor(56) = 56
var row = Math.floor((width - margin) / (frameWidth + spacing))
        = Math.floor((526 - (-1)) / (16 + 1))
        = Math.floor(31) = 31

However, this is still a bug !

Passing -1 as margin is hackish.
The reason why I added the + spacing in the formula is geometrical: because we divide by frameWidth + spacing to get the number of rows, we must consider that our "base pattern" is made of a sprite plus its spacing on one side (vertically or horizontally). Hence, given a spritesheet with no margin, we must add it a an extra spacing on one side in order to consider it a paving of our "base pattern".

If this spritesheet had a 2 pixels margin, we could not use the margin:-1 hack

[samme]

I think spacing implies the same length on the exterior edges as well. If that's absent, you do need a negative margin.

The definition is unclear, and it may not be the best one, but since it's always worked this way, I'd rather just change the documentation to explain it better. I'm open to feedback though.

margin = -1, spacing = 1

margin = 0, spacing = 1

margin = 1, spacing = 1

[Lucas-C]

I dislike the negative margin concept, but I don't have any strong argument against simply clarifying this in the documentation.

[samme]

I think even my understanding was wrong, and the docs are incorrect.

[samme]

So according to the code,

expected width = margin + rows * (frameWidth + spacing)

expected height = margin + cols * (frameHeight + spacing)

So that would give a layout something like

5px : mfsfs     margin=1 spacing=1
7px : mfssfss   margin=1 spacing=2
9px : mfsssfsss margin=1 spacing=3

[samme]

Corrected the docs (3fd72cc). Thanks @Lucas-C