Update packets: self release, mover blocks, spline facing, placement (#1324)

Five fixes to how `SMSG_UPDATE_OBJECT` is applied, each against the client's code:

- An out-of-range block never releases the active player (0x465f39–0x465f45).
- A movement block for the unit we move, the player or a possessed unit, is dropped (0x4670a5–0x4670bb).
- A create spline reads its three final-facing fields independently, in the order point, GUID, angle (0x47f0bf, 0x47f0e8, 0x47f0f9), so a server that sets two no longer breaks the packet.
- A game object is placed by its create block's position over its position fields (0x5f7046), and a re-create re-poses units only (0x46638c).
- The leave-world sweep's comment names the destroy path alone.

Byte-exact fixtures for the spline; each change has a test that fails without it.

Fixes #1307
Fixes #1308
Fixes #1309
Fixes #1310
Fixes #1311

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
(cherry picked from commit b816ff2f6054f496b7fb169a8acbd430323a98d3)

# Conflicts:
#	crates/benilla-app/src/ui_script/sweep.rs

Absorbed into the octo fork. Our tree has no ui_script/sweep.rs (upstream added it after our last sync), and the only change to it here was a comment; the file is dropped rather than orphaned. The five packet fixes apply unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
Sam Sung
2026-10-25 23:00:32 -06:00
committed by grom
co-authored by Claude Opus 5
parent 44aac335fd
commit b42cd7ef6b
5 changed files with 293 additions and 32 deletions
+29 -6
View File
@@ -21,8 +21,9 @@ use super::motion::{
trace_create_spline, trace_move_snap, wire_yaw, write_pose, SplineStopped, ROOT_APPLY_WIPE,
};
use super::{
merge_store_fields, FieldChanged, Guid, GuidIndex, NetCommands, NetEntity, NetHandlerApp,
ObjectStore, RemoteMotion, SelfGuid, SpeedChangeMessage, Spline, UnitMoveModes, UnitSpeeds,
merge_store_fields, ActiveMover, FieldChanged, Guid, GuidIndex, NetCommands, NetEntity,
NetHandlerApp, ObjectStore, RemoteMotion, SelfGuid, SpeedChangeMessage, Spline, UnitMoveModes,
UnitSpeeds,
};
/// `SMSG_GAMEOBJECT_CUSTOM_ANIM`, bridged to [`crate::go_anim`], which rejects `anim_id >= 4` and
@@ -84,6 +85,7 @@ pub(crate) struct Scene<'w, 's> {
self_guid: Res<'w, SelfGuid>,
real: Res<'w, Time<Real>>,
transforms: Query<'w, 's, &'static mut Transform>,
movers: Query<'w, 's, (), With<ActiveMover>>,
stores: Query<'w, 's, &'static mut ObjectStore>,
countdowns: Query<'w, 's, &'static mut crate::items::Countdowns>,
remote: Query<'w, 's, &'static mut RemoteMotion>,
@@ -223,6 +225,7 @@ fn on_object_move(In(ev): In<SessionEvent>, mut sc: Scene) {
&mut sc.commands,
&sc.index,
&mut sc.transforms,
&sc.movers,
);
}
}
@@ -310,8 +313,14 @@ fn on_object_destroyed(In(ev): In<SessionEvent>, mut sc: Scene) {
/// OUT_OF_RANGE takes the same deactivate as DESTROY in the reference, so a member walking out of
/// range also gets the snapshot and `CMSG_REQUEST_PARTY_MEMBER_STATS`.
///
/// The active player is never released: the reference compares each listed guid with
/// `ClntObjMgrGetActivePlayer` (`0x465f39` → `0x468550`, `0x465f3e`, `0x465f42`) and jumps past
/// `0x464920` (`0x465f45` → `0x465f54`), the deactivate included; a player's guid is outside the
/// second round's types (`0x465f19`, `0x465f28`), so no later pass reaches it.
fn on_objects_removed(In(ev): In<SessionEvent>, mut sc: Scene) {
if let SessionEvent::ObjectsRemoved(guids) = ev {
if let SessionEvent::ObjectsRemoved(mut guids) = ev {
guids.retain(|g| sc.self_guid.0 != Some(*g));
for guid in &guids {
let store = sc.index.0.get(guid).and_then(|e| sc.stores.get(*e).ok());
crate::ui_party::net::member_deactivated(*guid, &mut sc.group, store, &sc.net);
@@ -532,7 +541,7 @@ fn object_create(
trace_create_spline(guid, spline.as_ref());
let walk = spline.and_then(create_spline);
if let Some(&e) = index.0.get(&guid) {
// A re-create is a fresh snapshot: refresh identity and pose, drop any extrapolation.
// A re-create is a fresh snapshot: refresh identity and a unit's pose, drop any extrapolation.
commands.entity(e).insert(net).remove::<RemoteMotion>();
// The set is replaced whole; a `SMSG_FORCE_*_SPEED_CHANGE` in the same tick lands on top,
// since this insert applies before the next packet's handler (`apply::seam_tests`).
@@ -564,7 +573,11 @@ fn object_create(
commands.entity(e).remove::<Spline>();
}
}
write_pose(commands, transforms, e, position, placement);
// Only a unit takes the block's movement (type-mask bit 3, `0x46638c`); any other type
// reads the position and drops it (`0x4664c3`..`0x46656b`), so it stays where it stands.
if matches!(kind, EntityKind::Unit | EntityKind::Player) {
write_pose(commands, transforms, e, position, placement);
}
// The reference refreshes a live guid in place, notifying field watchers like any delta.
merge_fields(commands, stores, edges, e, guid, fields);
} else {
@@ -636,7 +649,10 @@ fn item_create(
}
}
/// An `SMSG_UPDATE_OBJECT` movement block: a new pose that supersedes any active path.
/// An `SMSG_UPDATE_OBJECT` movement block: a new pose that supersedes any active path. The unit
/// this client moves, our player or a possessed unit ([`ActiveMover`]), drops it: the reference
/// compares the block's guid with the active mover before it looks the object up (`0x4670a5`,
/// `0x4670ae`, `0x4670b5`), and returns on a match (`0x4670bb`).
fn object_move(
guid: u64,
position: [f32; 3],
@@ -644,8 +660,12 @@ fn object_move(
commands: &mut Commands,
index: &GuidIndex,
transforms: &mut Query<&mut Transform>,
movers: &Query<(), With<ActiveMover>>,
) {
if let Some(&e) = index.0.get(&guid) {
if movers.contains(e) {
return;
}
commands.entity(e).remove::<Spline>();
// A yaw only: the GameObjects that get one are transports, which their tick then owns.
write_pose(commands, transforms, e, position, wire_yaw(orientation));
@@ -1131,6 +1151,9 @@ fn speed_changed(
}
}
#[cfg(test)]
mod update_tests;
#[cfg(test)]
mod tests {
use super::*;
@@ -0,0 +1,144 @@
//! How `SMSG_UPDATE_OBJECT` blocks apply to the streamed world, through the handler table.
use super::*;
use benilla_assets::coords::wow_to_bevy;
const ME: u64 = 0x0000_0000_0000_0007;
const MOB: u64 = 0xF130_0000_1234_0001;
fn client() -> App {
crate::game_plugins::schedule_tests::headless_client()
}
/// An indexed entity for `guid`, as a create leaves it.
fn indexed(world: &mut World, guid: u64) -> Entity {
let e = world.spawn((Guid(guid), Transform::default())).id();
world.resource_mut::<GuidIndex>().0.insert(guid, e);
e
}
/// An out-of-range block lists the active player among others: the others are released, the
/// player is not (`0x465f39`..`0x465f45` skip `0x464920`).
#[test]
fn an_out_of_range_block_never_releases_the_active_player() {
let mut app = client();
let world = app.world_mut();
world.resource_mut::<SelfGuid>().0 = Some(ME);
let me = indexed(world, ME);
let mob = indexed(world, MOB);
super::super::handlers::dispatch(world, vec![SessionEvent::ObjectsRemoved(vec![MOB, ME])]);
let index = &world.resource::<GuidIndex>().0;
assert_eq!(index.get(&ME), Some(&me), "the player stays indexed");
assert!(world.get::<Guid>(me).is_some(), "and is not torn down");
assert!(world.get::<DespawnFade>(me).is_none(), "nor faded out");
assert!(
!index.contains_key(&MOB),
"control: another guid is released"
);
assert!(world.get::<DespawnFade>(mob).is_some());
}
fn moved(guid: u64) -> SessionEvent {
SessionEvent::ObjectMove {
guid,
position: [10.0, 20.0, 30.0],
orientation: 0.0,
}
}
/// A movement block names the unit this client moves, our player or a possessed unit, and is
/// dropped; any other indexed unit takes the pose (`0x4670a5`..`0x4670bb`). With control lost
/// the mover global is zero (`0x5fa600`), so the block applies.
#[test]
fn a_movement_block_for_the_unit_we_move_is_dropped() {
let mut app = client();
let world = app.world_mut();
let driven = indexed(world, ME);
world.entity_mut(driven).insert(ActiveMover);
let possessed = indexed(world, 0xF130_0000_0000_0002);
world.entity_mut(possessed).insert(ActiveMover);
let other = indexed(world, MOB);
let lost = indexed(world, 0xF130_0000_0000_0003);
super::super::handlers::dispatch(
world,
vec![
moved(ME),
moved(0xF130_0000_0000_0002),
moved(MOB),
moved(0xF130_0000_0000_0003),
],
);
let at = |world: &World, e: Entity| world.get::<Transform>(e).unwrap().translation;
assert_eq!(at(world, driven), Vec3::ZERO, "the player is not moved");
assert_eq!(at(world, possessed), Vec3::ZERO, "nor the possessed unit");
let pose = wow_to_bevy([10.0, 20.0, 30.0]);
assert_eq!(
at(world, other),
pose,
"control: another unit takes the pose"
);
assert_eq!(at(world, lost), pose, "control: an unsteered unit takes it");
}
fn create(guid: u64, kind: EntityKind, position: [f32; 3]) -> SessionEvent {
SessionEvent::ObjectCreate {
guid,
kind,
display_id: None,
position,
orientation: 0.0,
scale: 1.0,
speeds: None,
mover: None,
transport_progress: None,
transport: None,
spline: None,
fields: ObjectFields::default(),
}
}
/// A create of a known guid moves only a unit (type-mask bit 3, `0x46638c`); a GameObject's
/// block position is read and dropped (`0x4664c3`), so it keeps its place.
#[test]
fn a_recreate_re_poses_units_only() {
const GO: u64 = 0xF110_0000_0000_0009;
let mut app = client();
let (tx, _rx) = crossbeam_channel::unbounded();
app.insert_resource(NetCommands(tx));
let world = app.world_mut();
let at = |world: &World, guid: u64| {
let e = world.resource::<GuidIndex>().0[&guid];
world.get::<Transform>(e).unwrap().translation
};
super::super::handlers::dispatch(
world,
vec![
create(MOB, EntityKind::Unit, [1.0, 2.0, 3.0]),
create(GO, EntityKind::GameObject, [1.0, 2.0, 3.0]),
],
);
let first = wow_to_bevy([1.0, 2.0, 3.0]);
assert_eq!(
at(world, MOB),
first,
"control: the first create places both"
);
assert_eq!(at(world, GO), first);
super::super::handlers::dispatch(
world,
vec![
create(MOB, EntityKind::Unit, [10.0, 20.0, 30.0]),
create(GO, EntityKind::GameObject, [10.0, 20.0, 30.0]),
],
);
assert_eq!(
at(world, MOB),
wow_to_bevy([10.0, 20.0, 30.0]),
"a unit takes the new pose"
);
assert_eq!(at(world, GO), first, "a GameObject stays where it stands");
}
+5 -19
View File
@@ -1,6 +1,6 @@
//! The [`ServerPacket`] to [`SessionEvent`] mapping: pure functions of the packet, no state.
use crate::messages::{CastOutcome, MovementBlock, Object, ObjectFields, ObjectType, ServerPacket};
use crate::messages::{CastOutcome, Object, ObjectFields, ObjectType, ServerPacket};
use crate::wire::Vector3d;
use super::{EntityKind, MoveSpeeds, SessionEvent};
@@ -1124,9 +1124,10 @@ fn decode_objects(objects: Vec<Object>) -> Vec<SessionEvent> {
});
continue;
}
if let Some((position, orientation)) =
create_placement(object_type, &mask, &movement)
{
// The block's own position, for every type: the GameObject constructor copies it
// over `GAMEOBJECT_POS_X`..`FACING` (`0x5f7046`..`0x5f706b`), so the descriptor's
// copy never places anything.
if let Some((position, orientation)) = movement.position {
let display = display_id(object_type, &mask);
let scale = object_scale(object_type, &mask);
let speeds = movement.speeds.map(MoveSpeeds::from_wire);
@@ -1212,18 +1213,3 @@ fn object_scale(t: ObjectType, mask: &ObjectFields) -> f32 {
};
raw.filter(|s| *s > 0.0).unwrap_or(1.0)
}
/// A create's pose: a GameObject's `GAMEOBJECT_POS_*` when sent, else the movement block, since
/// vmangos never sets those fields on a transport. Gate on `gameobject_pos_sent`, not `.or`: a
/// create reads absent fields as zero, so `gameobject_position()` is `Some` even when unsent.
fn create_placement(
object_type: ObjectType,
mask: &ObjectFields,
movement: &MovementBlock,
) -> Option<(Vector3d, f32)> {
if object_type == ObjectType::GameObject && mask.gameobject_pos_sent() {
mask.gameobject_position()
} else {
movement.position
}
}
@@ -146,13 +146,17 @@ impl MovementBlock {
speeds = Some(s);
if flags & MOVEMENT_FLAG_SPLINE_ENABLED != 0 {
let spline_flags = read_u32_le(r)?;
// The final facing (`packet_builder.cpp:162-167`); a unit on a path faces along it.
// The final facing, three independent reads in the reference's order: a point
// (`0x47f0bf`), a guid (`0x47f0e8`), an angle (`0x47f0f9`). vmangos writes at most
// one (`packet_builder.cpp:162-167`); a unit on a path faces along it.
if spline_flags & SPLINE_FLAG_FINAL_POINT != 0 {
let _ = Vector3d::read(r)?;
}
if spline_flags & SPLINE_FLAG_FINAL_TARGET != 0 {
let _ = read_u64_le(r)?;
}
if spline_flags & SPLINE_FLAG_FINAL_ANGLE != 0 {
let _ = read_f32_le(r)?;
} else if spline_flags & SPLINE_FLAG_FINAL_TARGET != 0 {
let _ = read_u64_le(r)?;
} else if spline_flags & SPLINE_FLAG_FINAL_POINT != 0 {
let _ = Vector3d::read(r)?;
}
let time_passed_ms = read_u32_le(r)?;
let duration_ms = read_u32_le(r)?;
+106 -2
View File
@@ -9,8 +9,9 @@ use common::hx;
#[test]
fn update_object_fixture_decodes() {
// A unit create (0xAA), a gameobject create (0xBB) and an out-of-range block [0x10, 0x20].
let body = hx("03000000000301aa03200000000000000000cdd70bc6357e04c3f90fa74200000040000000000000803f0000e040000090400000000000000000db0f4940051700000000000000000000000000000008000000aa00000000000000090000000000c03f320000000301bb05000117810700bb000000000000002100000000000040630000000000c84200004843000096430000803f040200000001100120");
// A unit create (0xAA), a gameobject create (0xBB, a HAS_POSITION block at (100, 200, 300), as
// vmangos sends one) and an out-of-range block [0x10, 0x20].
let body = hx("03000000000301aa03200000000000000000cdd70bc6357e04c3f90fa74200000040000000000000803f0000e040000090400000000000000000db0f4940051700000000000000000000000000000008000000aa00000000000000090000000000c03f320000000301bb05400000c84200004843000096430000803f0117810700bb000000000000002100000000000040630000000000c84200004843000096430000803f040200000001100120");
let packet = messages::parse_server(messages::opcode::SMSG_UPDATE_OBJECT, &body).unwrap();
let events = decode(packet);
@@ -597,6 +598,109 @@ fn unit_living_block_surfaces_the_walk_it_is_already_on() {
}
}
/// The GameObject constructor copies the create block's position over `GAMEOBJECT_POS_X`..`FACING`
/// (`0x5f7046`..`0x5f706b`), so a create whose descriptor and block disagree is placed by the block.
#[test]
fn gameobject_create_is_placed_by_the_block_over_its_position_fields() {
let mut body = 1u32.to_le_bytes().to_vec();
body.push(0); // has_transport
body.push(2); // CREATE_OBJECT
write_packed_guid(0xBB, &mut body).unwrap();
body.push(5); // TypeId::GameObject
body.push(0x40); // HAS_POSITION
for v in [5000.0f32, 6000.0, 15.0, 2.0] {
body.extend_from_slice(&v.to_le_bytes()); // the block's pose
}
// Fields 8 (GAMEOBJECT_DISPLAYID) and 15..=18 (GAMEOBJECT_POS_X, _Y, _Z, FACING) in block 0.
body.push(1);
body.extend_from_slice(&((1u32 << 8) | (0xF << 15)).to_le_bytes());
body.extend_from_slice(&3015u32.to_le_bytes());
for v in [100.0f32, 200.0, 300.0, 1.0] {
body.extend_from_slice(&v.to_le_bytes()); // the descriptor's pose
}
let packet = messages::parse_server(messages::opcode::SMSG_UPDATE_OBJECT, &body).unwrap();
match decode(packet).as_slice() {
[SessionEvent::ObjectCreate {
position,
orientation,
display_id,
..
}] => {
assert_eq!(*display_id, Some(3015), "control: the fields were sent");
assert_eq!(*position, [5000.0, 6000.0, 15.0], "the block wins");
assert_eq!(*orientation, 2.0);
}
other => panic!("expected one ObjectCreate, got {other:?}"),
}
}
/// The create spline's final-facing bits are tested one by one, each field read in this order when
/// its bit is set: point (`0x10000`, 12 bytes, `0x47f0bf`), guid (`0x20000`, 8, `0x47f0e8`), angle
/// (`0x40000`, 4, `0x47f0f9`). The id, timing and path behind them land only if every set bit took
/// its bytes. Flags `0x50000` is the golden: the point (1, 2, 3), then the angle 1.5.
#[test]
fn create_spline_reads_each_final_facing_field_its_bit_names() {
// A unit create with a LIVING block on a spline: head and move flags, pose, speeds, then
// `spline flags`, the facing fields, timing (3000, 12000, id 42, 5 nodes), the 5 nodes and the
// final destination, and a descriptor behind it (UNIT_FIELD_DISPLAYID 1234).
fn create(spline_flags: &str, facing: &str) -> Vec<u8> {
hx(&[
concat!("01000000", "00", "02", "01aa", "03", "20"),
concat!("00004000", "00000000"),
concat!("000060410000a0410000f04100000000", "00000000"),
"000020400000e04000009040721c974000002040db0f4940",
spline_flags,
facing,
"b80b0000e02e00002a00000005000000",
concat!("000000000000a0410000f041", "000020410000a0410000f041"),
concat!("0000a0410000a0410000f041", "0000a041000020420000f041"),
concat!("0000a041000020420000f041", "0000a041000020420000f041"),
"050000000000000000000000000000000008000000d2040000",
]
.concat())
}
let point = "0000803f0000004000004040";
let guid = "1122334455667788";
let angle = "0000c03f";
for (name, flags, facing) in [
("point and angle", "00000500", [point, angle].concat()),
("point and guid", "00000300", [point, guid].concat()),
("guid and angle", "00000600", [guid, angle].concat()),
("all three", "00000700", [point, guid, angle].concat()),
("the point alone", "00000100", point.to_string()),
("the guid alone", "00000200", guid.to_string()),
("the angle alone", "00000400", angle.to_string()),
("none", "00000000", String::new()),
] {
let packet = messages::parse_server(
messages::opcode::SMSG_UPDATE_OBJECT,
&create(flags, &facing),
)
.unwrap();
match decode(packet).as_slice() {
[SessionEvent::ObjectCreate {
guid: 0xAA,
display_id,
spline,
..
}] => {
let s = spline.as_ref().expect("the spline");
assert_eq!(s.id, 42, "{name}: the id is read where the facing ends");
assert_eq!(s.time_passed_ms, 3_000, "{name}");
assert_eq!(s.duration_ms, 12_000, "{name}");
assert_eq!(s.path.len(), 3, "{name}");
assert_eq!(
*display_id,
Some(1234),
"{name}: the fields behind it parse"
);
}
other => panic!("{name}: expected one ObjectCreate, got {other:?}"),
}
}
}
#[test]
fn unit_living_block_without_the_spline_flag_carries_no_walk() {
let mut body = 1u32.to_le_bytes().to_vec();