Bevy hot-reloading hierarchies cause panic (most of the time) #7529
Comments
Testare
added
C-Bug
alice-i-cecile
added
A-UI
|
I've done some debugging on my own, and while I haven't found the exact root cause, wanted to document what I've found so far. Process: I first checked out this repository, and migrated my code more-or-less to this intermediate bevy version, and used the local repo for the dependency,and injected some println statements into the bevy_scene crate, and modified the scene file to get different results. So what I've found is basically this: When the scene is hot-reloaded, it uses an entity map to map from scene entity numbers to the actual world entity numbers. So long as the real-world entity ID for the entity the DynamicSceneBundle was added to is NOT a key in the entity map (I.E. not an entity ID used in the scene), hot-reloading works fine. So, when the DynamicScene is spawned, all entities without a parent already defined set are added as children to the entity with the DynamicSceneBundle (I'll call this the root entity). This is all well and good. But when the scene is reload, the entities keep this Parent(RootEntity) component. Then the code that updates these components to point at actual Entity ID's instead of the Scene-internal Entity ID's passes through, and sees these Parent(RootEntity) components, and updates them with the Entity Map to point at some other entity (It seems that if it can't find that entity ID in the entity map, it leaves it alone). For example: At runtime, the DynamicScene entity is created with entity ID 2. It then spawns the scene. While in the scene they're defined as 0, 1, and 2, at runtime the actual world entity IDs become 4, 5, and 6. An entity map is created (0->4, 1->5, 2->6). Then the Parent/Children components are updated by this. So while in the scene code, entity 1 defines 0 as its parent entity, the actual entity 5's parent component is updated to point at entity 4. This is all well and good: This preserves the relationship as modeled in the scene file. Then, because this is how things are done, all the nodes in the scene without parents already defined are added as children of the DynamicScene entity that spawned them. So entity 4 (scene entity 0) gains a parent component with a value of 2. Then we update the scene asset. The scene is reloaded, but luckily we have the entity map to preserve the mapping of scene entities to real entities. We adjust these entities and their components. The scene must once again correct the parent and children components so that instead of pointing to values like 0, 1, and 2, they point ot the appropriate entities 4, 5, and 6. But hold up! Scene entity 0 (real entity 4) has a parent component too now, that points at entity 2, the dynamic scene root. But I guess the code that applies the entity map to these components doesn't realize that is what this is. It sees the parent component pointing to 2, and updates it to point at entity 6. Now we have entity 4 which is simultaneously the child and parent of entity 6. Malformed hierachy! Code panics. It seems that if the entity map does not contain a key for this match, it fails quitely and allows the parent component to remain unchanged. I.E. if in the previous example, the DynamicScene entity was entity 3 instead of 2. Then there would be no entry in the map to change it to, and the updates work just fine. This explains why it works sometimes and not others. tldr; It looks like code that updates parent/children components in scenes doesn't work well with the fact that entities without parents in the scene are added as children of the entity with the DynamicSceneBundle |
|
Yeah, that's what it looks like. The Parent Component that is being added to scene entities is being looped through the MapEntities code. I will say, the fact that I'm able to more or less understand what the code is doing is a great compliment to the people who wrote and maintained it. Looks better than most code I come across in my job! XD That said, I'm not entirely sure how to fix this bug ^-^' Having scene entities be children of their spawning entity seems to idiomatic and logical that I don't think breaking this link is the solution. Moreover, this problem only occurs when a scene is reloaded. Conversely, changing the contract of MapEntities to allow some sort of metadata to pass through seems difficult, especially in figured out what the metadata should be and how this metadata is passed. I suppose we could add some sort of property to the Parent component to mark it as one that does not bear updating through MapEntities, but then you have to consider the implications for all the other possible uses of MapEntities, such as networking. The solution I think sounds most reasonable is to have some sort of mechanism for a scene to remove certain components from an entity when the scene is reloaded. Perhaps which components are removed this way could be a property of the Reflect trait? If we can remove the Parent component of all entities that don't have parent components defined in the scene, we can just re-add them to the appropriate parent on reload. That's still kinda a hefty operation though. |
|
Turns out I was perhaps overreacting about how complicated the fix would have to be ^-^' |
|
Sorry I didn't find it before, but I believe this issue might be a duplicate of #4793 |
Fix a bug with scene reload. (This is a copy of #7570 but without the breaking API change, in order to allow the bugfix to be introduced in 0.10.1) When a scene was reloaded, it was corrupting components that weren't native to the scene itself. In particular, when a DynamicScene was created on Entity (A), all components in the scene without parents are automatically added as children of Entity (A). But if that scene was reloaded and the same ID of Entity (A) was a scene ID as well*, that parent component was corrupted, causing the hierarchy to become malformed and bevy to panic. *For example, if Entity (A)'s ID was 3, and the scene contained an entity with ID 3 This issue could affect any components that: * Implemented `MapEntities`, basically components that contained references to other entities * Were added to entities from a scene file but weren't defined in the scene file - Fixes #7529 The solution was to keep track of entities+components that had `MapEntities` functionality during scene load, and only apply the entity update behavior to them. They were tracked with a HashMap from the component's TypeID to a vector of entity ID's. Then the `ReflectMapEntities` struct was updated to hold a function that took a list of entities to be applied to, instead of naively applying itself to all values in the EntityMap. (See this PR comment #7570 (comment) for a story-based explanation of this bug and solution) - Components that implement `MapEntities` added to scene entities after load are not corrupted during scene reload.
Fix a bug with scene reload. (This is a copy of bevyengine#7570 but without the breaking API change, in order to allow the bugfix to be introduced in 0.10.1) When a scene was reloaded, it was corrupting components that weren't native to the scene itself. In particular, when a DynamicScene was created on Entity (A), all components in the scene without parents are automatically added as children of Entity (A). But if that scene was reloaded and the same ID of Entity (A) was a scene ID as well*, that parent component was corrupted, causing the hierarchy to become malformed and bevy to panic. *For example, if Entity (A)'s ID was 3, and the scene contained an entity with ID 3 This issue could affect any components that: * Implemented `MapEntities`, basically components that contained references to other entities * Were added to entities from a scene file but weren't defined in the scene file - Fixes bevyengine#7529 The solution was to keep track of entities+components that had `MapEntities` functionality during scene load, and only apply the entity update behavior to them. They were tracked with a HashMap from the component's TypeID to a vector of entity ID's. Then the `ReflectMapEntities` struct was updated to hold a function that took a list of entities to be applied to, instead of naively applying itself to all values in the EntityMap. (See this PR comment bevyengine#7570 (comment) for a story-based explanation of this bug and solution) - Components that implement `MapEntities` added to scene entities after load are not corrupted during scene reload.
Bevy version
0.9.1
What you did
I was setting up the UI for the game, and decided that it would be convenient to have the UI as a scene so I could adjust the UI at runtime. So I created a start up system to create some initial stub nodes, used
bevy_editor_plsto save the scene, and adjusted the scene file, including removing unnecessary entities and adjusting entity numbering. I then added this scene with a DynamicSceneBundle to a node with a NodeBundle, set AssetPlugin for hot reloading, ran the code, and began adjusting the scene file while the program was running.[Link to the repo]
What went wrong
When the program was running, I would adjust the scene file (Such as changing a background color's red value), and the program would panic and close! It would claim that the reason was a malformed hierarchy.
So I put a system in so that whenever the window is resized (random trigger), it would print out information about the hierarchy of the entities loaded by the scene, and the hierarchy looks just fine! More telling, the initial load of the scene always works just fine.
The thing is, every now and then, it would actually work just fine. Sometimes when the program is running, I can adjust the scene as much as I want, and the program won't panic at all. This might have something to do with entity ordering. It seems like this happens more often when the EditorPlugin code is uncommented?
I assume this has something to do with how components like Parent/Child are loaded by the asset loader on hot-reload, since obviously the references in the scene file won't be the same entity references in the scene.
Additional information
Initial hierarchy
I added a system to verify that initially the hierarchy is correctly set up, that prints all entities with Node components, their names, their parents and children. This system runs when the window is resized, so you have to resize the window after the scene loads for it to work.
Results look like this: (Window UI Root is the node that the DynamicSceneBundle is loaded onto)
The entity labels are different numbers from the ones in the scene file, which seems logical, and the hierarchy matches with the shape I was expecting.
Github repo
Luckily it was pretty early on in the project, so I commited the initial commit to this public repo https://github.com/Testare/khaldron/tree/1e4a3ab4153a7df4abcb4a9b14bd440dc115c826
You should be able to check it out to reproduce it. The bug also exists on the current main (commit 2), it just has some extra stuff that isn't relevant to figuring out the bug.
Default panic message:
RUST_BACKTRACE=1RUST_BACKTRACE=full
The text was updated successfully, but these errors were encountered: