Trouv / bevy_ecs_ldtk

ECS-friendly ldtk plugin for bevy, leveraging bevy_ecs_tilemap
Other
687 stars 79 forks source link

[Feature request] Level respawn API #96

Closed bardt closed 2 years ago

bardt commented 2 years ago

I miss a way to restart the level (or all levels in the set). What I looking for is a convenient way to:

  1. Request a respawn
  2. Receive LevelEvent::Despawned so I could run my cleanup logic
  3. Receive LevelEvent::Spawned so I could run my initialization logic

Proposed solution

I would imagine an Event consumer of the library can send, which is handled by a couple of systems in an order:

  1. despawn_system, removing current or requested level from LevelSet. Run before LdtkSystemLabel::PreSpawn.
  2. respawn_system, inserting current or requested level to LevelSet. Run after LdtkSystemLabel::PreSpawn but before CoreStage::Update.

This way apply_level_set system can react to the removal of the level and trigger Despawned event; there will be 1 frame gap until respawn though.

I implemented roughly this idea in this branch

Trouv commented 2 years ago

Thanks for the issue! Will give this some thought for sure

Trouv commented 2 years ago

It turns out that the plugin needs a new respawning mechanism internally due to hot reloading being broken (#108). I'm thinking of making a new Respawn component. This component can be inserted on a Handle<LdtkAsset> entity, a Handle<LdtkLevel> entity, or an EntityInstance entity, and when it is, the plugin will respawn those entities/their children.

When inserted on the world/level, the appropriate systems will respawn them by:

  1. despawning their descendants if they have children
  2. spawning them normally if they don't have children, and removing the Respawn component.

The plugin can insert this on the world on AssetEvent::Modified for hot reloading.

The approach for EntityInstance will have to be slightly different, and I haven't completely thought through the implementation yet. This use case may be worth putting off until a future release, but I think it would be a reasonable feature eventually.