grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Survive a net entity despawned before attach's queue is applied

Browse Source 
grom's 06:23 session died 183 s in on first world entry
(Diagnostics/crash-1792671984.txt): insert::<GroundShade> on 4431v0, "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 mechanism is structural, not a one-off. 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 while
attach_entity_visuals' query runs: the row looks alive, the whole visual is
built for it, and the despawn lands first at the sync point. The panicking
command form then kills the client for an entity the server had already taken
away.

So every write this system makes onto the net entity root is now a try_
command. add_child was the one that needed more than a rename: it queues its
command on the *parent*, so the conform node and RigPose::anchor_for's bone
anchors are related at spawn with ChildOf instead — the relationship hook
tolerates a missing target where a command on it does not.

The test builds a CommandQueue the way the existing rig test does, despawns the
root before applying it, and asserts the entity stays gone. It fails with
grom's exact panic text when any single write is put back, which is how I know
it bites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-10-22 07:41:59 -06:00
co-authored by Claude Opus 5
parent b4a5d3b38f
commit f434cc9916
2 changed files with 110 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/entities/attach/mod.rs
+104 -33
View File
@@ -116,8 +116,8 @@ fn setup_skinned_instance(
// loops.
commands
.entity(entity)
.insert(rig)
.remove::<benilla_world::rig_palette::RigStarved>();
.try_insert(rig)
.try_remove::<benilla_world::rig_palette::RigStarved>();
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::<benilla_world::rig_anim::AnimParked>();
.try_insert(AnimationTransitions::new())
.try_remove::<benilla_world::rig_anim::AnimParked>();
}
_ => {
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::<benilla_world::rig_anim::AnimParked>();
.try_remove::<benilla_world::rig_anim::AnimParked>();
}
}
}
@@ -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::<GroundShade>` 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::<super::mount::MountChild>();
commands
.entity(entity)
.try_remove::<super::mount::MountChild>();
}
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::<benilla_world::interior::ContainmentAttach>();
.try_remove::<benilla_world::interior::ContainmentAttach>();
}
}
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::<benilla_world::model_fade::UnitAppearFade>();
.try_remove::<benilla_world::model_fade::UnitAppearFade>();
}
// 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::<super::equipment::Reattached>();
.try_insert(super::live_display::AppliedDisplay(net.display_id))
.try_remove::<super::equipment::Reattached>();
}
}
@@ -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::<RigPalettes>()
.init_resource::<Assets<SkinnedMeshInverseBindposes>>();
let mut ibps = app
.world_mut()
.remove_resource::<Assets<SkinnedMeshInverseBindposes>>()
.unwrap();
let d = one_bone(&mut ibps);
let mut world = std::mem::take(app.world_mut());
let mut palettes = world.remove_resource::<RigPalettes>().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);
}
}
crates/benilla-world/src/rig_anim/pose.rs
+6 -1
View File
@@ -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)
}