handle relative teleports correctly and fix entity chunk indexing warnings

azalea-client/src/plugins/packet/game/mod.rs

@@ -1,10 +1,9 @@
 mod events;
 
-use std::{collections::HashSet, ops::Add, sync::Arc};
+use std::{collections::HashSet, sync::Arc};
 
 use azalea_core::{
     game_type::GameMode,
-    math,
     position::{ChunkPos, Vec3},
 };
 use azalea_entity::{
@@ -425,68 +424,12 @@ impl GamePacketHandler<'_> {
 
             **last_sent_position = **position;
 
-            fn apply_change<T: Add<Output = T>>(base: T, condition: bool, change: T) -> T {
+            p.relative
-                if condition { base + change } else { change }
+                .apply(&p.change, &mut position, &mut direction, &mut physics);
-            }
-
-            let new_x = apply_change(position.x, p.relative.x, p.change.pos.x);
-            let new_y = apply_change(position.y, p.relative.y, p.change.pos.y);
-            let new_z = apply_change(position.z, p.relative.z, p.change.pos.z);
-
-            let new_y_rot = apply_change(
-                direction.y_rot,
-                p.relative.y_rot,
-                p.change.look_direction.y_rot,
-            );
-            let new_x_rot = apply_change(
-                direction.x_rot,
-                p.relative.x_rot,
-                p.change.look_direction.x_rot,
-            );
-
-            let mut new_delta_from_rotations = physics.velocity;
-            if p.relative.rotate_delta {
-                let y_rot_delta = direction.y_rot - new_y_rot;
-                let x_rot_delta = direction.x_rot - new_x_rot;
-                new_delta_from_rotations = new_delta_from_rotations
-                    .x_rot(math::to_radians(x_rot_delta as f64) as f32)
-                    .y_rot(math::to_radians(y_rot_delta as f64) as f32);
-            }
-
-            let new_delta = Vec3::new(
-                apply_change(
-                    new_delta_from_rotations.x,
-                    p.relative.delta_x,
-                    p.change.delta.x,
-                ),
-                apply_change(
-                    new_delta_from_rotations.y,
-                    p.relative.delta_y,
-                    p.change.delta.y,
-                ),
-                apply_change(
-                    new_delta_from_rotations.z,
-                    p.relative.delta_z,
-                    p.change.delta.z,
-                ),
-            );
-
-            // apply the updates
-
-            physics.velocity = new_delta;
-
-            (direction.y_rot, direction.x_rot) = (new_y_rot, new_x_rot);
-
-            let new_pos = Vec3::new(new_x, new_y, new_z);
-            if new_pos != **position {
-                **position = new_pos;
-            }
-
             // old_pos is set to the current position when we're teleported
             physics.set_old_pos(&position);
 
             // send the relevant packets
-
             commands.trigger(SendPacketEvent::new(
                 self.player,
                 ServerboundAcceptTeleportation { id: p.id },
@@ -494,8 +437,8 @@ impl GamePacketHandler<'_> {
             commands.trigger(SendPacketEvent::new(
                 self.player,
                 ServerboundMovePlayerPosRot {
-                    pos: new_pos,
+                    pos: **position,
-                    look_direction: LookDirection::new(new_y_rot, new_x_rot),
+                    look_direction: *direction,
                     // this is always false
                     on_ground: false,
                 },
@@ -852,6 +795,8 @@ impl GamePacketHandler<'_> {
     }
 
     pub fn teleport_entity(&mut self, p: &ClientboundTeleportEntity) {
+        debug!("Got teleport entity packet {p:?}");
+
         as_system::<(Commands, Query<(&EntityIdIndex, &InstanceHolder)>)>(
             self.ecs,
             |(mut commands, mut query)| {
@@ -862,26 +807,28 @@ impl GamePacketHandler<'_> {
                     return;
                 };
 
-                let new_pos = p.change.pos;
+                let relative = p.relative.clone();
-                let new_look_direction = LookDirection {
+                let change = p.change.clone();
-                    x_rot: (p.change.look_direction.x_rot as i32 * 360) as f32 / 256.,
+
-                    y_rot: (p.change.look_direction.y_rot as i32 * 360) as f32 / 256.,
-                };
                 commands.entity(entity).queue(RelativeEntityUpdate::new(
                     instance_holder.partial_instance.clone(),
                     move |entity| {
-                        let mut position = entity.get_mut::<Position>().unwrap();
+                        let entity_id = entity.id();
-                        if new_pos != **position {
+                        entity.world_scope(move |world| {
-                            **position = new_pos;
+                            let mut query =
-                        }
+                                world.query::<(&mut Physics, &mut LookDirection, &mut Position)>();
-                        let position = *position;
+                            let (mut physics, mut look_direction, mut position) =
-                        let mut look_direction = entity.get_mut::<LookDirection>().unwrap();
+                                query.get_mut(world, entity_id).unwrap();
-                        if new_look_direction != *look_direction {
+                            let old_position = *position;
-                            *look_direction = new_look_direction;
+                            relative.apply(
-                        }
+                                &change,
-                        // old_pos is set to the current position when we're teleported
+                                &mut position,
-                        let mut physics = entity.get_mut::<Physics>().unwrap();
+                                &mut look_direction,
-                        physics.set_old_pos(&position);
+                                &mut physics,
+                            );
+                            // old_pos is set to the current position when we're teleported
+                            physics.set_old_pos(&old_position);
+                        });
                     },
                 ));
             },
@@ -914,11 +861,7 @@ impl GamePacketHandler<'_> {
                     instance_holder.partial_instance.clone(),
                     move |entity_mut| {
                         let mut physics = entity_mut.get_mut::<Physics>().unwrap();
-                        let new_pos = physics.vec_delta_codec.decode(
+                        let new_pos = physics.vec_delta_codec.decode(&new_delta);
-                            new_delta.xa as i64,
-                            new_delta.ya as i64,
-                            new_delta.za as i64,
-                        );
                         physics.vec_delta_codec.set_base(new_pos);
                         physics.set_on_ground(new_on_ground);
 
@@ -968,17 +911,13 @@ impl GamePacketHandler<'_> {
                     instance_holder.partial_instance.clone(),
                     move |entity_mut| {
                         let mut physics = entity_mut.get_mut::<Physics>().unwrap();
-                        let new_pos = physics.vec_delta_codec.decode(
+                        let new_position = physics.vec_delta_codec.decode(&new_delta);
-                            new_delta.xa as i64,
+                        physics.vec_delta_codec.set_base(new_position);
-                            new_delta.ya as i64,
-                            new_delta.za as i64,
-                        );
-                        physics.vec_delta_codec.set_base(new_pos);
                         physics.set_on_ground(new_on_ground);
 
                         let mut position = entity_mut.get_mut::<Position>().unwrap();
-                        if new_pos != **position {
+                        if new_position != **position {
-                            **position = new_pos;
+                            **position = new_position;
                         }
 
                         let mut look_direction = entity_mut.get_mut::<LookDirection>().unwrap();

azalea-client/src/test_utils/simulation.rs

@@ -106,6 +106,9 @@ impl Simulation {
     pub fn tick(&mut self) {
         tick_app(&mut self.app);
     }
+    pub fn update(&mut self) {
+        self.app.update();
+    }
 
     pub fn minecraft_entity_id(&self) -> MinecraftEntityId {
         self.component::<MinecraftEntityId>()

azalea-client/tests/move_and_despawn_entity.rs

@@ -0,0 +1,57 @@
+use azalea_client::test_utils::prelude::*;
+use azalea_core::{
+    position::{ChunkPos, Vec3},
+    resource_location::ResourceLocation,
+};
+use azalea_entity::metadata::Cow;
+use azalea_protocol::{
+    common::movements::{PositionMoveRotation, RelativeMovements},
+    packets::{
+        ConnectionProtocol,
+        game::{ClientboundRemoveEntities, ClientboundTeleportEntity},
+    },
+};
+use azalea_registry::{DataRegistry, DimensionType, EntityKind};
+use azalea_world::MinecraftEntityId;
+use bevy_ecs::query::With;
+
+#[test]
+fn test_move_and_despawn_entity() {
+    init_tracing();
+
+    let mut simulation = Simulation::new(ConnectionProtocol::Game);
+    simulation.receive_packet(make_basic_login_packet(
+        DimensionType::new_raw(0),
+        ResourceLocation::new("azalea:overworld"),
+    ));
+
+    for x in 0..=10 {
+        simulation.receive_packet(make_basic_empty_chunk(ChunkPos::new(x, 0), (384 + 64) / 16));
+    }
+    simulation.tick();
+
+    simulation.receive_packet(make_basic_add_entity(EntityKind::Cow, 123, (0.5, 64., 0.5)));
+    simulation.tick();
+
+    simulation.receive_packet(ClientboundTeleportEntity {
+        id: MinecraftEntityId(123),
+        change: PositionMoveRotation {
+            pos: Vec3::new(16., 0., 0.),
+            delta: Vec3::ZERO,
+            look_direction: Default::default(),
+        },
+        relative: RelativeMovements::all_relative(),
+        on_ground: true,
+    });
+    simulation.receive_packet(ClientboundRemoveEntities {
+        entity_ids: vec![MinecraftEntityId(123)],
+    });
+    simulation.tick();
+
+    // make sure it's despawned
+    let mut cow_query = simulation.app.world_mut().query_filtered::<(), With<Cow>>();
+    let cow_iter = cow_query.iter(simulation.app.world());
+    assert_eq!(cow_iter.count(), 0, "cow should be despawned");
+
+    simulation.tick();
+}

azalea-entity/src/plugin/indexing.rs

@@ -132,16 +132,17 @@ pub fn update_entity_chunk_positions(
     instance_container: Res<InstanceContainer>,
 ) {
     for (entity, pos, instance_name, mut entity_chunk_pos) in query.iter_mut() {
-        // TODO: move this inside of the if statement so it's not called as often
-        let instance_lock = instance_container.get(instance_name).unwrap();
-        let mut instance = instance_lock.write();
-
         let old_chunk = **entity_chunk_pos;
         let new_chunk = ChunkPos::from(*pos);
         if old_chunk != new_chunk {
             **entity_chunk_pos = new_chunk;
 
             if old_chunk != new_chunk {
+                let Some(instance_lock) = instance_container.get(instance_name) else {
+                    continue;
+                };
+                let mut instance = instance_lock.write();
+
                 // move the entity from the old chunk to the new one
                 if let Some(entities) = instance.entities_by_chunk.get_mut(&old_chunk) {
                     entities.remove(&entity);
@@ -163,7 +164,10 @@ pub fn insert_entity_chunk_position(
     instance_container: Res<InstanceContainer>,
 ) {
     for (entity, pos, world_name) in query.iter() {
-        let instance_lock = instance_container.get(world_name).unwrap();
+        let Some(instance_lock) = instance_container.get(world_name) else {
+            // entity must've been despawned already
+            continue;
+        };
         let mut instance = instance_lock.write();
 
         let chunk = ChunkPos::from(*pos);
@@ -213,13 +217,13 @@ pub fn remove_despawned_entities_from_indexes(
 
         let mut instance = instance_lock.write();
 
-        // if the entity has no references left, despawn it
+        // if the entity is being loaded by any of our clients, don't despawn it
         if !loaded_by.is_empty() {
             continue;
         }
 
         // remove the entity from the chunk index
-        let chunk = ChunkPos::from(*position);
+        let chunk = ChunkPos::from(position);
         match instance.entities_by_chunk.get_mut(&chunk) {
             Some(entities_in_chunk) => {
                 if entities_in_chunk.remove(&entity) {
@@ -247,9 +251,21 @@ pub fn remove_despawned_entities_from_indexes(
                 }
             }
             _ => {
-                debug!(
+                let mut found_in_other_chunks = HashSet::new();
-                    "Tried to remove entity {entity:?} from chunk {chunk:?} but the chunk was not found."
+                for (other_chunk, entities_in_other_chunk) in &mut instance.entities_by_chunk {
-                );
+                    if entities_in_other_chunk.remove(&entity) {
+                        found_in_other_chunks.insert(other_chunk);
+                    }
+                }
+                if found_in_other_chunks.is_empty() {
+                    warn!(
+                        "Tried to remove entity {entity:?} from chunk {chunk:?} but the chunk was not found and the entity wasn't in any other chunks."
+                    );
+                } else {
+                    warn!(
+                        "Tried to remove entity {entity:?} from chunk {chunk:?} but the chunk was not found. Entity found in and removed from other chunk(s): {found_in_other_chunks:?}"
+                    );
+                }
             }
         }
         // remove it from the uuid index

azalea-entity/src/plugin/mod.rs

@@ -46,11 +46,6 @@ impl Plugin for EntityPlugin {
             Update,
             (
                 (
-                    // remove_despawned_entities_from_indexes is done again here to correctly
-                    // handle the case where an entity is spawned and then the world is removed at
-                    // the same time (like with ClientboundStartConfiguration).
-                    indexing::remove_despawned_entities_from_indexes
-                        .in_set(EntityUpdateSet::Deindex),
                     indexing::update_entity_chunk_positions,
                     indexing::insert_entity_chunk_position,
                 )

azalea-entity/src/vec_delta_codec.rs

@@ -1,4 +1,4 @@
-use azalea_core::position::Vec3;
+use azalea_core::{delta::PositionDelta8, position::Vec3};
 
 #[derive(Debug, Clone, Default)]
 pub struct VecDeltaCodec {
@@ -10,7 +10,11 @@ impl VecDeltaCodec {
         Self { base }
     }
 
-    pub fn decode(&self, x: i64, y: i64, z: i64) -> Vec3 {
+    pub fn decode(&self, delta: &PositionDelta8) -> Vec3 {
+        let x = delta.xa as i64;
+        let y = delta.ya as i64;
+        let z = delta.za as i64;
+
         if x == 0 && y == 0 && z == 0 {
             return self.base;
         }

azalea-protocol/src/common/movements.rs

@@ -1,8 +1,11 @@
-use std::io::{self, Cursor, Write};
+use std::{
+    io::{self, Cursor, Write},
+    ops::Add,
+};
 
 use azalea_buf::{AzBuf, AzaleaRead, AzaleaWrite, BufReadError};
-use azalea_core::{bitset::FixedBitSet, position::Vec3};
+use azalea_core::{bitset::FixedBitSet, math, position::Vec3};
-use azalea_entity::LookDirection;
+use azalea_entity::{LookDirection, Physics, Position};
 
 /// The updated position, velocity, and rotations for an entity.
 ///
@@ -32,6 +35,60 @@ impl RelativeMovements {
     pub fn all_absolute() -> Self {
         RelativeMovements::default()
     }
+    pub fn all_relative() -> Self {
+        RelativeMovements {
+            x: true,
+            y: true,
+            z: true,
+            y_rot: true,
+            x_rot: true,
+            delta_x: true,
+            delta_y: true,
+            delta_z: true,
+            rotate_delta: true,
+        }
+    }
+
+    pub fn apply(
+        &self,
+        change: &PositionMoveRotation,
+        position: &mut Position,
+        direction: &mut LookDirection,
+        physics: &mut Physics,
+    ) {
+        let new_position = Vec3::new(
+            apply_change(position.x, self.x, change.pos.x),
+            apply_change(position.y, self.y, change.pos.y),
+            apply_change(position.z, self.z, change.pos.z),
+        );
+
+        let new_look_direction = LookDirection::new(
+            apply_change(direction.y_rot, self.y_rot, change.look_direction.y_rot),
+            apply_change(direction.x_rot, self.x_rot, change.look_direction.x_rot),
+        );
+
+        let mut new_delta = physics.velocity;
+        if self.rotate_delta {
+            let y_rot_delta = direction.y_rot - new_look_direction.y_rot;
+            let x_rot_delta = direction.x_rot - new_look_direction.x_rot;
+            new_delta = new_delta
+                .x_rot(math::to_radians(x_rot_delta as f64) as f32)
+                .y_rot(math::to_radians(y_rot_delta as f64) as f32);
+        }
+        let new_delta = Vec3::new(
+            apply_change(new_delta.x, self.delta_x, change.delta.x),
+            apply_change(new_delta.y, self.delta_y, change.delta.y),
+            apply_change(new_delta.z, self.delta_z, change.delta.z),
+        );
+
+        **position = new_position;
+        *direction = new_look_direction;
+        physics.velocity = new_delta;
+    }
+}
+
+fn apply_change<T: Add<Output = T>>(base: T, condition: bool, change: T) -> T {
+    if condition { base + change } else { change }
 }
 
 impl AzaleaRead for RelativeMovements {

azalea-protocol/src/packets/game/c_teleport_entity.rs

@@ -9,6 +9,6 @@ pub struct ClientboundTeleportEntity {
     #[var]
     pub id: MinecraftEntityId,
     pub change: PositionMoveRotation,
-    pub relatives: RelativeMovements,
+    pub relative: RelativeMovements,
     pub on_ground: bool,
 }