Trouv / bevy_ecs_ldtk

ECS-friendly ldtk plugin for bevy, leveraging bevy_ecs_tilemap
Other
674 stars 76 forks source link

Add indexing for layer/entity data by iid #251

Open Trouv opened 11 months ago

Trouv commented 11 months ago

A redesign of the asset types has made the process of accessing loaded level data more clear and correct, for both internal-levels and external-levels projects. Since LdtkProject stores a mapping of level iids to level handles/their indices in the project (along with other metadata), accessing levels by iid is a constant-time operation. Providing similar indexing of entity iids/layer iids would provide many benefits for us and for users. Some of these benefits include..

However, unlike mapping level iids to levels, we cannot provide a mapping of entities to layers or layers to levels inside the asset types. This is because, in the external levels case, this data is not available to the LdtkProject. We would need create these indexes in a normal bevy system and store it in an actual bevy resource, and probably provide a SystemParam API for end-users. The index could look something like this:


struct EntityIndex {
    layer_iid: LayerIid,
    index: usize,
}

struct LayerIndex {
    level_iid: LevelIid,
    index: usize,
}

#[derive(Resource)]
struct LdtkIndex {
    entity_map: HashMap<EntityIid, EntityIndex>,
    layer_map: HashMap<LayerIid, LayerIndex>,
    level_map: HashMap<LevelIid, Handle<LdtkProject>>,
}

Then a system param that grabs this resource and Assets<LdtkProject> and #[cfg(feature = "external_levels")] Assets<LdtkExternalLevel> could provide methods like..

No cloning (apart from the strings used in the iids), and constant-time operations all the way through the tree. It might be worth putting some of this behind feature flags due to the memory and compilation costs that some users may want to opt out of. However, some of the benefits this could provide to internal systems makes me hesitant to put all of it behind a feature flag.

This should come after #250 and #249. It is also worth noting that this is based off work and discussion done in #195 that could not be completed due to the asset type design at the time. Thanks @adtennant