diff --git a/crates/benilla-app/src/entities/attach/mod.rs b/crates/benilla-app/src/entities/attach/mod.rs index d4e7e51d..16f4ddbc 100644 --- a/crates/benilla-app/src/entities/attach/mod.rs +++ b/crates/benilla-app/src/entities/attach/mod.rs @@ -116,8 +116,8 @@ fn setup_skinned_instance( // loops. commands .entity(entity) - .insert(rig) - .remove::(); + .try_insert(rig) + .try_remove::(); slot } // Nothing to skin: no starvation marker, or `heal_rig_starved` would rebuild the body for a @@ -127,7 +127,7 @@ fn setup_skinned_instance( // Table full: static bind-pose parts until `heal_rig_starved` rebuilds with a slot. commands .entity(entity) - .insert(benilla_world::rig_palette::RigStarved); + .try_insert(benilla_world::rig_palette::RigStarved); 0 } }; @@ -137,7 +137,7 @@ fn setup_skinned_instance( if let Some(drive) = benilla_world::rig_anim::GlobalSeqDrive::new_rig(&anims.global_bones, nbones) { - commands.entity(entity).insert(drive); + commands.entity(entity).try_insert(drive); } } Some(RigBuild { pose, slot }) @@ -172,14 +172,14 @@ fn arm_sequence_clock( } } } - commands.entity(entity).insert(( + commands.entity(entity).try_insert(( player, AnimationGraphHandle(anims.graph.clone()), anims.clone(), )); match kind { EntityKind::GameObject if go_state_machine => { - commands.entity(entity).insert(( + commands.entity(entity).try_insert(( // Cross-fades each transition over the clip's blend-in time, from the seed. transitions, crate::go_anim::GoAnim::default(), @@ -192,20 +192,20 @@ fn arm_sequence_clock( EntityKind::Corpse => { commands .entity(entity) - .insert(AnimationTransitions::new()) - .remove::(); + .try_insert(AnimationTransitions::new()) + .try_remove::(); } _ => { commands .entity(entity) - .insert(( + .try_insert(( // Cross-fades over each clip's blend-in time, so a gait change eases. AnimationTransitions::new(), AnimDriver::default(), )) // A rebuilt rig is born live: a stale park marker would desync the LOD gate's // edge-triggered bookkeeping. - .remove::(); + .try_remove::(); } } } @@ -219,6 +219,20 @@ struct RigBuild { /// Attach a visual to each net entity without one, our own avatar included. The net bridge or the /// player controller owns the entity's transform; only its scale is set here. +/// +/// **Every write onto the net entity root here is a `try_` command**, and that is load-bearing, +/// not caution. A command queue is applied at the schedule's next sync point, not when the system +/// returns, so a destroy handled by a system that ran *earlier* in the same frame is still only +/// *queued* when this query runs: the row looks alive, the work is done for it, and the despawn +/// lands first at the sync point. The panicking form then kills the client for an entity the +/// server had already taken away. grom's 2026-10-22 06:23 session died exactly there, 183 s in on +/// first world entry (`Diagnostics/crash-1792671984.txt`): `insert::` on 4431v0, +/// "the entity with ID 4431v0 is invalid; its index now has generation 1", one frame after a +/// 4301 ms world-entry hitch during which the server sent a burst of creates and destroys. +/// +/// The children this system spawns are not exposed the same way — a fresh entity cannot be stale — +/// but a command *queued on the parent* is, which is why the conform node is related at spawn and +/// `RigPose::anchor_for` spawns its anchors with `ChildOf` rather than calling `add_child`. #[allow(clippy::type_complexity)] pub(super) fn attach_entity_visuals( mut commands: Commands, @@ -365,7 +379,9 @@ pub(super) fn attach_entity_visuals( if let Ok(mut ec) = commands.get_entity(child) { ec.despawn(); } - commands.entity(entity).remove::(); + commands + .entity(entity) + .try_remove::(); } let kind = match net.kind { EntityKind::GameObject => ModelKind::GameObject, @@ -376,15 +392,16 @@ pub(super) fn attach_entity_visuals( // The model-local interior fold point; the classifier applies the entity scale. let bake_center = dm.map(|d| d.bake_center_local).unwrap_or(Vec3::ZERO); // One ground-shade state per object (the reference's `[obj+0xe0]`), the same 2.5/0.5 - // MCSH chase for every kind; `insert_if_new` so a rebuild keeps the ramped state. + // MCSH chase for every kind; `insert_if_new` so a rebuild keeps the ramped state. The + // `try_` is the crash in this function's own doc comment: this was the line. commands .entity(entity) - .insert_if_new(benilla_world::entity_shade::GroundShade::default()); + .try_insert_if_new(benilla_world::entity_shade::GroundShade::default()); // Held items share the body's interior verdict (the reference aliases the wearer's // collector into each item, `0x718960`), so they fold at the body's centre. commands .entity(entity) - .insert(benilla_world::interior::BodyBakeCenter(bake_center)); + .try_insert(benilla_world::interior::BodyBakeCenter(bake_center)); // The light node's attach mode (`[node+0x90]` bit 13, set from the TYPEMASK at // `0x613e10`/`0x670db0`, dispatched at `0x6a86d0`): a GameObject by containment at its // bounds centre (`0x6a8c10`), a unit by down-ray at its position (`0x6a8a20`). @@ -392,12 +409,12 @@ pub(super) fn attach_entity_visuals( EntityKind::GameObject => { commands .entity(entity) - .insert(benilla_world::interior::ContainmentAttach); + .try_insert(benilla_world::interior::ContainmentAttach); } _ => { commands .entity(entity) - .remove::(); + .try_remove::(); } } let object = WorldObject { @@ -435,9 +452,11 @@ pub(super) fn attach_entity_visuals( }, Transform::default(), Visibility::default(), + // Related at spawn rather than by `add_child`: that queues its + // command on the *parent*, which may be gone by the sync point. + ChildOf(entity), )) .id(); - commands.entity(entity).add_child(node); joints_root = node; } setup_skinned_instance( @@ -507,7 +526,7 @@ pub(super) fn attach_entity_visuals( for m in &d.markers { markers.entry(m.ident).or_insert((m.bone, m.offset)); } - commands.entity(entity).insert(super::BoneAttach { + commands.entity(entity).try_insert(super::BoneAttach { points: d .attachments .iter() @@ -526,7 +545,7 @@ pub(super) fn attach_entity_visuals( if spine.is_some() || head.is_some() { commands .entity(entity) - .insert(crate::creature_anim::BodyTwist::new(spine, head)); + .try_insert(crate::creature_anim::BodyTwist::new(spine, head)); } } // A character model carries every hair, facial and body geoset, and is shared by its @@ -592,7 +611,7 @@ pub(super) fn attach_entity_visuals( .map(|c| bevy::camera::primitives::Aabb::from_min_max(c.bounds_min, c.bounds_max)) .or_else(|| union_aabb(model.unwrap_or_default().iter().filter_map(|p| p.aabb))); if let Some(aabb) = election_bound { - commands.entity(entity).insert(super::ModelBound(aabb)); + commands.entity(entity).try_insert(super::ModelBound(aabb)); } // A billboard card rides its live joint, so card anchors resolve here, over the spawn // loop's geoset predicate: a hidden variant's card bone spawns no anchor. @@ -659,11 +678,11 @@ pub(super) fn attach_entity_visuals( if unit_will_fade { commands .entity(entity) - .insert(benilla_world::model_fade::UnitAppearFade::Pending { since: now }); + .try_insert(benilla_world::model_fade::UnitAppearFade::Pending { since: now }); } else if reattached { commands .entity(entity) - .remove::(); + .try_remove::(); } // Size is `OBJECT_FIELD_SCALE_X` alone: the server already folds the DBC scale into // it (vmangos `Unit::GetScaleForDisplayId`, `Unit.cpp:9579`), and the reference @@ -678,25 +697,25 @@ pub(super) fn attach_entity_visuals( if let (EntityKind::Player, Some(e)) = (net.kind, equipment) { commands .entity(entity) - .insert(super::equipment::AppliedEquipment(*e)); + .try_insert(super::equipment::AppliedEquipment(*e)); } // The camera's framing pivot, model-local, before scale. - commands.entity(entity).insert(CameraPivot { + commands.entity(entity).try_insert(CameraPivot { height_local: dm.map(|d| d.pivot_height_local).unwrap_or(0.0), // The pivot's drop while swimming, the reference's third preset `cam+0x124` // (`0x50ccf6`); 0 without a Swim sequence. swim_drop_local: dm.map(|d| d.swim_pivot_drop_local).unwrap_or(0.0), }); // The overhead fallback for a model with no PlayerName attachment (`0x608640`). - commands.entity(entity).insert(super::OverheadFallback( + commands.entity(entity).try_insert(super::OverheadFallback( dm.map(|d| d.bbox_z_local).unwrap_or(0.0), )); // The chat bubble's height: the Stand box's Z, not the posed PlayerName attachment. - commands.entity(entity).insert(super::StandBoxHeight( + commands.entity(entity).try_insert(super::StandBoxHeight( dm.map(|d| d.stand_box_z_local).unwrap_or(0.0), )); // The selection ring's model-local radius, scaled by the unit's scale. - commands.entity(entity).insert(SelectionRadius( + commands.entity(entity).try_insert(SelectionRadius( dm.map(|d| d.ground_radius_local).unwrap_or(0.0), )); // Emitters ride their host bone's anchor, origin rebased into the bone frame @@ -780,7 +799,7 @@ pub(super) fn attach_entity_visuals( } // The pose lands last, with every anchor the build resolved into it. if let Some(rb) = skin { - commands.entity(entity).insert(rb.pose); + commands.entity(entity).try_insert(rb.pose); } // A GameObject with a hull collides; an anchored transport stays Kinematic, as its arm // labelled it, since its body moves every frame. @@ -791,7 +810,7 @@ pub(super) fn attach_entity_visuals( } else { RigidBody::Static }; - commands.entity(entity).insert((body, col)); + commands.entity(entity).try_insert((body, col)); // This lane bypasses the streamer's attach queue, so it stamps the collider // set itself. collider_epoch.bump(); @@ -857,14 +876,14 @@ pub(super) fn attach_entity_visuals( .map_or(0, |s| s.0.unit_mount_display_id()); commands .entity(entity) - .insert(super::mount::AppliedMount(applied)); + .try_insert(super::mount::AppliedMount(applied)); } commands .entity(entity) - .insert(VisualAttached) + .try_insert(VisualAttached) // The display the visual was built with, for `refresh_live_display` to diff. - .insert(super::live_display::AppliedDisplay(net.display_id)) - .remove::(); + .try_insert(super::live_display::AppliedDisplay(net.display_id)) + .try_remove::(); } } @@ -970,4 +989,56 @@ mod tests { *app.world_mut() = world; } } + /// The crash in [`attach_entity_visuals`]'s doc comment, as a test: the writes this system + /// queues onto a net entity root must survive that root being despawned before the queue is + /// applied. A destroy handled earlier in the same frame is only *queued* while this system + /// runs, so the row it iterates can already be doomed — and the queue is applied afterwards, + /// at the sync point, in the order the systems ran. + /// + /// Written against the queue rather than a schedule because that is where the fault lived: + /// grom's session died inside `Commands::apply`, not inside the system. + #[test] + fn a_root_despawned_before_the_queue_is_applied_does_not_take_the_client_with_it() { + use benilla_world::rig_palette::RigPalettes; + + let mut app = App::new(); + app.init_resource::() + .init_resource::>(); + let mut ibps = app + .world_mut() + .remove_resource::>() + .unwrap(); + let d = one_bone(&mut ibps); + let mut world = std::mem::take(app.world_mut()); + let mut palettes = world.remove_resource::().unwrap(); + + let entity = world.spawn_empty().id(); + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + // The rig, a bone anchor and the ground shade: the writes the real system makes onto + // the root, including the component that named the crash and the `add_child` that + // used to queue a command *on* it. + let mut build = + setup_skinned_instance(&mut commands, &mut palettes, entity, entity, &d, true) + .expect("the rig builds"); + assert!( + build.pose.anchor_for(&mut commands, entity, 0).is_some(), + "the one bone anchors" + ); + commands + .entity(entity) + .try_insert_if_new(benilla_world::entity_shade::GroundShade::default()); + commands.entity(entity).try_insert(VisualAttached); + } + // The destroy the server sent, applied first because its system ran first. + world.despawn(entity); + queue.apply(&mut world); + + assert!( + world.get_entity(entity).is_err(), + "the entity stays gone — a tolerated write must not resurrect it" + ); + world.insert_resource(palettes); + } } diff --git a/crates/benilla-world/src/rig_anim/pose.rs b/crates/benilla-world/src/rig_anim/pose.rs index d41a7f4c..1660bd2f 100644 --- a/crates/benilla-world/src/rig_anim/pose.rs +++ b/crates/benilla-world/src/rig_anim/pose.rs @@ -125,10 +125,15 @@ impl RigPose { }, Visibility::default(), RigAnchor { rig, bone }, + // Related at spawn, not by `commands.entity(joints_root).add_child(anchor)`: + // `add_child` queues its command on the *parent*, and the parent here is a net + // entity root that a destroy may already have queued a despawn for — the + // relationship hook tolerates a missing target, the command on it does not. + // See `entities::attach::attach_entity_visuals`. + ChildOf(self.joints_root), )) .vis_chain_only() .id(); - commands.entity(self.joints_root).add_child(anchor); self.anchors.push((bone, anchor)); Some(anchor) }