Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add indexing for layer/entity data by iid #251

Open
Trouv opened this issue Oct 18, 2023 · 0 comments
Open

Add indexing for layer/entity data by iid #251

Trouv opened this issue Oct 18, 2023 · 0 comments

Comments

@Trouv
Copy link
Owner

Trouv commented Oct 18, 2023

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..

  • easier and quicker access to EntityInstance/LayerInstance/Level data in normal bevy systems (this defeats one of the biggest cons of using the "blueprint pattern" for LDtk entity processing) without any clones
  • redundancy of the LayerMetadata component, which could be completely removed in favor of quickly grabbing the same data directly from the asset
  • no more performance reason for processing levels via triple-layer iteration over the asset data, so we could redesign internal systems and schedules to be more modular

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..

  • fn get_level(_: &LevelIid) -> Option<LoadedLevel>
  • fn get_level_for_layer(_: &LayerIid) -> Option<LoadedLevel>
  • fn get_layer(_: &LayerIid) -> Option<&LayerInstance>
  • fn get_level_for_entity(_: &EntityIid) -> Option<LoadedLevel>
  • fn get_layer_for_entity(_: &EntityIid) -> Option<&LayerInstance>
  • fn get_entity(_: &EntityIid) -> Option<&EntityInstance>

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant