search

Trouv / bevy_ecs_ldtk

ECS-friendly ldtk plugin for bevy, leveraging bevy_ecs_tilemap
Other
664 stars 75 forks source link

Respect pivot in tile layers #162

Open geieredgar opened 1 year ago

geieredgar commented 1 year ago

This should fix #152.

Tiles which are not aligned to the grid are put into own layers, one layer for each encountered alignment offset. They also only span the part of the grid in which tiles are found. Another separate layer is added that contains the entities for the int grid cells.

Trouv commented 1 year ago

Finally starting to look at this. It's kinda scary changing stuff like this. Here's an example why - check out the platformer example, the tops of the ladders don't seem to be acting as ladders. Not sure why yet

geieredgar commented 1 year ago

I've found the bug, it was this line:

max: IVec2::new(layer_instance.c_wid, layer_instance.c_hei),

It should have been

max: IVec2::new(layer_instance.c_wid - 1, layer_instance.c_hei - 1),

During debugging I've tried out several things and am now wondering if it would be better to have one entity that corresponds to the layer (like before, but without the TileMapBundle, because we may need several of those). Then the tile-maps for each sub-layer would be a children of the layer entity, and the tile entities would also be children of the layer entity instead of the tile map entity. This would restore the previous behavior that all tiles of a layer are children of the same entity.

I am also wondering if we should add spatial bundles only to tile entities created from the IntGridCell, because all other entities should probably only be used for rendering.

Trouv commented 1 year ago

During debugging I've tried out several things and am now wondering if it would be better to have one entity that corresponds to the layer (like before, but without the TileMapBundle, because we may need several of those). Then the tile-maps for each sub-layer would be a children of the layer entity, and the tile entities would also be children of the layer entity instead of the tile map entity. This would restore the previous behavior that all tiles of a layer are children of the same entity.

This makes sense to me - though I'm not sure off the top of my head if bevy_ecs_tilemap requires tile entities to be children of tilemaps?

If that hierarchy is required, I think that's fine. That would be a breaking change for many users, but it sounds like a logical hierarchy to me and it's not like we're a stable library.

I am also wondering if we should add spatial bundles only to tile entities created from the IntGridCell, because all other entities should probably only be used for rendering.

IntGridCells and tiles w/ tile metadata. I think giving it to EVERY tile is a holdover from when we used batch-spawning, since they all had to have the same bundle. So, unless there's some other use case for that I'm forgetting, that makes sense to me too.