feat: add `Respawn` component and rework schedule using it

Closes #96 Closes #108

This is a pretty major change internally. If you're reading this and want to help me out, please update your games to this branch and let me know here whether or not there are any issues. I would like this to be the final PR before releasing 0.4, and I would like to release 0.4 very soon.

bevy_ecs_ldtk = { git = "https://github.com/Trouv/bevy_ecs_ldtk", branch = "feat/respawn" }

The scheduling has been completely reworked, and some systems have been significantly changed as a result. However, this is definitely an improvement. Many frame-delay issues are addressed by this rework, and hot reloading works again. This change was assisted by a new Respawn component, which can work on world entities and level entities. As a bonus, users can use the Respawn component in their games. I imagine this will mostly be used to restart levels.

The main point of this rework is to be more careful about having stage separation between commands-sensitive, order-sensitive systems. If one adds a particular component to entities, and another filters for those additions, there needs to be a stage barrier between them. Most importantly, if one despawns entities, and another spawns similar entities, there needs to be stage barrier between them. This last point was the main cause for a lot of bugs since updating to 0.8 and the bevy_ecs_tilemap rewrite.

One drawback here is that there's a new stage between update and post-update: LdtkStage::Clean. It's important that this goes before PostUpdate, since it despawns bevy_ecs_tilemap entities, which need to be further cleaned up in PostUpdate. It's also important that this goes after Update so that users can use Respawn and Worldly components without experiencing frame-delay or undefined behavior, and without having to worry about doing anything special with their scheduling. This isn't ideal, but I think it's a small price to pay. Besides, stageless is just around the corner.

Use cases targeted

Spawning LdtkWorldBundle in startup system
- Selecting levels with LevelSelection
- Selecting levels with LevelSet
Spawning LdtkWorldBundle in non-startup system (State::on_enter)
- Selecting levels with LevelSelection
- Selecting levels with LevelSet
Spawning LdtkWorldBundle with an already-loaded LdtkAsset handle
Hot reloading levels
Respawning world with Respawn component
Respawning level with Respawn component

New schedule (updated c5984fded3e875a42d9daa1e238b3ad964ac8a7a)

Stage PreUpdate
- LdtkAsset processor system
- Reads AssetEvent::Modified and adds Respawn components to entities with those handles
- level spawn system
- For any new placeholder level entities, or any that have Respawn components, spawn their contents normally
Stage LdtkStage::ProcessApi (between update and post update)
- worldly adoption system
- Have the grandparents of worldly entities adopt them (no change)
- Needs to be after Update for frame delay reasons
- LevelSelection system
- Updates LevelSet components according to LevelSelection values
- Needs to be after Update and before the LevelSet system for frame delay reasons
- LevelSet system
- For any world entities whose LevelSets have changed, or that have Respawn components, spawn/despawn placeholder level entities as children accordingly
- Respawn cleanup system
- Exclusive system at the end of the stage
- For Handle<LdtkAsset> entities with Respawn components, despawns all of their children with Worldly or Handle<LdtkLevel> components
- For Handle<LdtkLevel> entities with Respawncomponents, despawns their descendants.
- Needs to be after the LevelSet system for frame delay reasons, but before PostUpdate to appease bevy_ecs_tilemap cleanup

Known issues (to be fixed or promoted)

When using Respawn on a world where a Worldly entity has the only handle to a particular asset via #[sprite_bundle(...)], that asset unloads.

Found another issue - LevelSelection and LevelSet updates being detected in CoreStage::First is problematic for users updating LevelSelection in a normal CoreStage::Update system. In this scenario, the spawning of new levels ends up occurring before the despawning of the old levels. This causes frame-delay and lots of visual bugs.

One thing that should be done anyway is making Respawn a little less sensitive to spawning. We should change systems that do spawning on Respawn entities so that they only do that spawning if the despawning has already occurred. This should force despawning to occur before spawning and get rid of the visual bugs. However, this doesn't address the frame-delay.

Putting the level selection updates in CoreStage::First prevents frame-delay for AssetEvent changes. In order to prevent frame-delay for LevelSelection and LevelSet changes during CoreStage::Update, we'd need to make sure those changes are caught and cause despawns to occur before CoreStage::PostUpdate. So, LdtkStage::Clean seems like a good place to put them (and maybe we should rename that stage at that point). However, then we have frame delay for AssetEvent-related spawning, unless we decide to put them in both.

I don't think the overhead that putting them in both would introduce is worth it just to save a frame-delay on AssetEvent::Created. AssetEvent::Modified already has a frame delay anyway.

So with the LevelSelection and LevelSet systems in LdtkStage::Clean, and having accepted a frame delay on AssetEvents, that actually frees us up to put the LdtkAsset system anywhere between CoreStage::Last and CoreStage::Update. I think I'll put it in CoreStage::PreUpdate to make things a little simpler.

Let's see if that all works like it does in my head.

This plan sort of worked, but I made some more tweaks. In particular, I've realized that processing the LDtk asset events so heavily wasn't necessary. Now the process_ldtk_assets system does significantly less than it used to. The apply_level_selection and apply_level_set systems that actually trigger level spawning are good enough on their own to spawn levels when an asset has finished loading, or when a new world entity with an old ldtk asset is spawned.

I've added some updates to really stabilize those two systems. They now only act if the asset is loaded, are more idempotent, and are more calculated in their Respawn usage.

I've also added another stage for these two systems: LdtkStage::LevelSelection. They shouldn't be in the same stage as the respawn cleanup system after all, since they do remove some Respawn components with commands. However, I think I'll get rid of this stage. Two new stages is simply too much. These two systems will unfortunately be placed in CoreStage::Update. Now that level selection changes are more stable, it won't matter if users accidentally perform those actions after the level selection systems. They will experience frame delays, but if they really want to avoid them that bad then they'll be able to see in the docs what system ordering they need to employ.

Placing these level selection systems is pretty difficult. I think all of the options are either bad or a compromise:

anywhere between CoreStage::PostUpdate and CoreStage::First (wrapping around) is bad. This causes spawning of new levels to occur before despawning of old levels when changing the LevelSelection, causing visual bugs.
LdtkStage::Clean is bad because the level selection systems remove Respawn components from the world entity, so there needs to be a commands barrier between them and the clean system.
CoreStage::Update is a compromise. The big drawback here is that users need to carefully order systems that add children to levels before the level selection systems. Not doing so causes crashes.
A new stage LdtkStage::LevelSelection between CoreStage::Update and LdtkStage::Clean is a compromise. There's no bugs here, but it hurts performance by adding a new stage entirely.
CoreStage::PreUpdate, after the level spawning systems, is a compromise. Updating LevelSelection and LevelSet changes will have frame delay here, unless the user does some crazy specific system ordering. In fact, the changes are pretty much as delayed as they could possibly be. If a user just updates a level selection in CoreStage::Update, the old levels won't despawn until 1 update later, and the new levels won't spawn until 2 updates later.

Right now I think that, out of the 3 compromise options, CoreStage::PreUpdate is the best. Possibility of crashing if you're not careful is far worse than occasional frame delay. Adding a new stage that eats up a little time of every update is far worse than occasional frame delay. I'm open to other opinions on this, but I think it is the lesser of 3 evils.

I think all of the options are either bad or a compromise

Would it be possible to leave this choice up to the user by setting a field on LdtkPlugin?

I'm not too familiar with the concept of stageless, but if I understand correctly it would entirely solve this ordering issue, right? I'm concerned about choosing CoreStage::PreUpdate now, and then on the next upgrade having game behavior change because these frame delays have disappeared.

Actually, there was another option that I wasn't giving enough consideration. I've now put the level selection systems in LdtkStage::ProcessApi (renamed from LdtkStage::Clean), and made the clean_respawn_entities system an exclusive system that occurs at the end of that stage.

This puts the commands barrier between the level selection systems and the clean system that prevented me from putting them in the same stage before. Now, the user can insert/change LevelSelection, LevelSet, Worldly and Respawn in a normal CoreStage::Update system and expect no frame delay.

Making it an exclusive system does raise some performance flags, but I don't think it will actually make a difference in this case. The scheduling for clean_respawn_entities before made it the only system running at that time. In other words, it was practically exclusive already, so there's no reduced parallelism in making it actually exclusive.

Would it be possible to leave this choice up to the user by setting a field on LdtkPlugin?

Yes, that's a good idea, but there's no compromise in the changes that I've made tonight so I don't think there's a need to make it optional anymore.

I'm not too familiar with the concept of stageless, but if I understand correctly it would entirely solve this ordering issue, right?

Yes. In stageless, you'll be able to place these "commands barriers" wherever you want, pretty much. Honestly, I'm not sure what the scheduling of plugins is going to look like after stageless. I think it's going to change things pretty drastically for plugin authors. Positive changes though.

Trouv / bevy_ecs_ldtk