colinvella / phaser-tilemap-plus

Tilemap animations, physics, events and custom property enhancements for Tiled JSON map files
MIT License
71 stars 5 forks source link

`tilemap.removeTile` etc operations don't modify `plus.tileAnimations` #4

Open nemoDreamer opened 6 years ago

nemoDreamer commented 6 years ago

I realize this is a nice-to-have, but would make using animations more plug-and-play:

When calling this.tilemap.removeTile(x, y, layer) (or "swap", "randomize", etc...), the corresponding this.tilemap.plus.tileAnimations[*].tileLocations[*] doesn't get "corrected".

I've currently got a crude external approach:

// keys: 23-27
// spear: 28
// dagger: 29
// money: 30
if (itemsTile && itemsTile.index >= 23 && itemsTile.index <= 30) {
    this.tilemap.removeTile(
        this.player.x,
        this.player.y,
        this.itemsLayer,
    );

    // remove rotating "key" animation?
    if (itemsTile.index >= 23 && itemsTile.index <= 27) {
        const anim = this.tilemap.plus.animation.tileAnimations.find(
            animation => animation.frames[0].tileId === 22,
        );
        const ind = anim.tileLocations.findIndex(
            location =>
                location.x === this.player.x &&
                location.y === this.player.y,
        );

        anim.tileLocations.splice(ind, 1);
    }
}

But would be sweet to have removeTile patched to remove "any tileLocations at given coordinates where tileAnimations' frames contains index".

And while typing this, I just realized that I could just as well override TileMap.prototype.removeTile and make this less clunky...

Maybe I'll send along a PR 😜

colinvella commented 6 years ago

You seem to have a good grasp of the code already! :)

As you figured out yourself, the system builds a structure that keeps track of the animated tiles. If you are confident you can submit a PR, please do go ahead! I think we should also look at other layer modification code besides removeTile(..).

nemoDreamer commented 6 years ago

Yup, I mentioned "swap", "randomize" etc above.

Tilemap.swap will need to check for removal and potentially add a new tileLocation (and maybe even a whole new tileAnimation... (haven't checked if you add all available ones in the tileset, regardless of whether it's used.))

Tilemap.random will only need to correct tileLocation coordinates.

But yeah, nothing trivial in those 2... Then apart from the obvious "add", there's "copy", "paste", "replace",...

Damn... I understand why tile animations weren't made part of the core lib yet.

Would be a whole lot easier if it were the Tile object itself that was responsible for managing its own animations...

colinvella commented 6 years ago

Yeah, it would have been easier if it was encapsulated in each tile somehow. Also, when I initially coded the animation system, I didn't give much thought to map modification once it was loaded. I'll tackle this in the next update.