Trouv / bevy_ecs_ldtk

ECS-friendly ldtk plugin for bevy, leveraging bevy_ecs_tilemap
Other
627 stars 73 forks source link

[feat] Add an option to disable spawning of some/all IntGrid entities #272

Closed focustense closed 7 months ago

focustense commented 7 months ago

I've added a few different IntGrids to track different types of tile metadata and noticed that the result is essentially a new entity hierarchy, tilemap, etc. for each grid under the level entity.

I appreciate that we can make the IntGrid tiles invisible, but in my particular case, I really have no use for these entities at all, and I'm a little concerned about their effect on performance. I'm not using them for AutoLayers, I don't need to see them in the game world (because I can see them very easily in LDtk itself), and there is a lot of redundant data being spawned - hundreds of entities each with dozens of components, many of which are going to trigger some tilemap systems and other systems to run despite being invisible to the eye. It's compounded when I'm loading up to 4 level neighbors.

Would it be possible to add something to the LdtkSettings, or wherever makes the most sense, that allows either ignoring all IntGrid layers (except AutoLayers, I guess) or selectively whitelisting/blacklisting the layers we're interested/not interested in?

I'd like to also point out that when using IntGrid as metadata - which is kind of the point, AutoLayers weren't implemented until much later - it is usually not faster to query the ECS than it is to just query the level data in the LDtk project, because the ECS is not indexed by any useful key. Whereas, in the LDtk project itself, if we know the grid coordinates of the tile we want to query, and also know the grid size, then it is simple and cheap math to translate that into an index and no problem at all to do this a few hundred times per frame.

(P.S. I totally get that it will be useful for some users to get a whole entity tree for those layers, maybe they want to run additional systems or spawn new entities based on it, etc., and those are better optimized in ECS style. That's why I'm asking for it to be a choice.)

(P.P.S. If you think it's a useful request, but don't have time/are busy with other priorities, I'm willing to take a crack at a PR if you could give me a few pointers re: where the magic happens.)

Trouv commented 7 months ago

This is reasonable. I think a simple skip_layers: Vec<String> in LdtkSettings storing the identifiers of the layers to not spawn would be perfectly reasonable, easy to implement, and should still be relevant as this plugin changes its spawning processes in the future. level::spawn_level could probably just .filter() for layers not-contained in this list in its main loop. Would that satisfy your needs?