Fix attachment sync lifecycle and far chunk snapshots

crates/horizon-client/src/game/attachment_sync.rs

@@ -67,9 +67,7 @@ impl Game<'_> {
         }
         holders
             .into_iter()
-            .filter(|(holder, changes)| {
-                !changes.is_empty() || matches!(holder, Holder::Player(_))
-            })
+            .filter(|(holder, changes)| !changes.is_empty() || matches!(holder, Holder::Player(_)))
             .map(|(holder, changes)| Message {
                 holder,
                 replace: true,
@@ -84,9 +82,6 @@ impl Game<'_> {
     /// who see them.
     pub(super) fn flush_attachment_sync(&mut self, host: &mut LanHost) {
         let hub = &mut host.hub;
-        if crate::mod_api::mods().is_empty() {
-            hub.attachment_snapshots.clear();
-        }
         for snapshot in std::mem::take(&mut hub.attachment_snapshots) {
             let AttachmentSnapshot { player, holder } = snapshot;
             let Some(guest) = hub.guests.iter().find(|g| g.uuid == player) else {

crates/horizon-client/src/game/lan.rs

@@ -1282,6 +1282,7 @@ impl Game<'_> {
             .then(|| horizon_client::lod_net::Service::new(net.lod_distance, biome_names.clone()));
         let lan = Lan::start_with(sync.clone(), net)?;
         let mut hub = Hub::new(lan, sync, encoder, config);
+        hub.sync_attachments = !crate::mod_api::mods().is_empty();
         hub.flat_world = self.world_flat;
         hub.debug_world = self.world_debug;
         let port = hub.lan.port;

crates/horizon-client/src/game/remote.rs

@@ -1891,6 +1891,7 @@ impl Game<'_> {
                     self.score.board = Default::default();
                     self.clear_waypoints();
                     if let Some(r) = self.remote.as_mut() {
+                        r.attachments.clear();
                         r.players.clear();
                         r.tab_players.clear();
                         r.listed.clear();
@@ -3030,6 +3031,7 @@ impl Game<'_> {
                 // replaces ServerReconfigScreen.
                 if let Some(r) = self.remote.as_mut() {
                     r.reconfiguring = None;
+                    r.attachments.clear();
                 }
                 // handleLogin makes a new MultiPlayerGameMode (carried index 0) and a
                 // new LocalPlayer (slot 0, no effects, riding nothing: the server

crates/horizon-client/src/lan/guest.rs

@@ -204,6 +204,8 @@ pub struct Guest {
     // Chunks.
     pub center: Option<(i32, i32)>,
     pub sent_chunks: crate::fast_hash::FxHashSet<(i32, i32)>,
+    /// Sent as far-ring columns and needing a near snapshot on promotion.
+    pub far_sent_chunks: crate::fast_hash::FxHashSet<(i32, i32)>,
     pub chunk_queue: Vec<(i32, i32)>,
     /// The far ring's chunks still to send, past the view distance (`lan::far`), closest first.
     pub far_queue: Vec<(i32, i32)>,
@@ -417,6 +419,7 @@ impl Guest {
             load_timeout: CLIENT_LOADED_TIMEOUT,
             center: None,
             sent_chunks: Default::default(),
+            far_sent_chunks: Default::default(),
             chunk_queue: Vec::new(),
             far_queue: Vec::new(),
             unacknowledged: 0,

crates/horizon-client/src/lan/hub.rs

@@ -210,15 +210,18 @@ fn take_tracking(
     g: &mut Guest,
     out: &mut Vec<TrackingBatch>,
     snapshots: &mut Vec<AttachmentSnapshot>,
+    sync_attachments: bool,
 ) {
     let events = g.tracker.take_events();
     if !events.is_empty() {
-        let started = events.iter().filter(|e| e.start);
-        queue_snapshots(
-            snapshots,
-            g.uuid,
-            started.map(|e| Holder::Entity(e.entity as u32)),
-        );
+        if sync_attachments {
+            let started = events.iter().filter(|e| e.start);
+            queue_snapshots(
+                snapshots,
+                g.uuid,
+                started.map(|e| Holder::Entity(e.entity as u32)),
+            );
+        }
         out.push(TrackingBatch {
             player: g.uuid,
             name: g.name.clone(),
@@ -236,6 +239,8 @@ pub struct Hub {
     /// The holders players started seeing, for the game to send their synced
     /// attachments (`flush_attachment_sync`).
     pub attachment_snapshots: Vec<AttachmentSnapshot>,
+    /// Whether native mods can produce synced attachments.
+    pub sync_attachments: bool,
     /// Players a native mod hid (`native_hide_player`), by UUID, on or off:
     /// only players of at least that permission level see them, in the
     /// player list, the world and their joining and leaving.
@@ -583,6 +588,7 @@ impl Hub {
             native_menu_events: Vec::new(),
             native_tracking: Vec::new(),
             attachment_snapshots: Vec::new(),
+            sync_attachments: false,
             hidden: Default::default(),
             teams: Vec::new(),
             nicknames: std::collections::HashMap::new(),
@@ -1431,11 +1437,13 @@ impl Hub {
     fn send_level_info(&mut self, i: usize, ctx: &TickCtx, load_start: bool) {
         let spawn = ctx.spawn;
         let g = &self.guests[i];
-        queue_snapshots(
-            &mut self.attachment_snapshots,
-            g.uuid,
-            [Holder::Level, Holder::Player(g.uuid)],
-        );
+        if self.sync_attachments {
+            queue_snapshots(
+                &mut self.attachment_snapshots,
+                g.uuid,
+                [Holder::Level, Holder::Player(g.uuid)],
+            );
+        }
         g.send(&C::InitializeBorder(super::border::initialize(ctx.border)));
         g.send(&self.time_packet(ctx.world, ctx.clock_rate));
         g.send(&self.default_spawn(spawn));
@@ -1731,7 +1739,12 @@ impl Hub {
     /// are kept in `native_tracking`.
     pub fn collect_tracking(&mut self) {
         for g in &mut self.guests {
-            take_tracking(g, &mut self.native_tracking, &mut self.attachment_snapshots);
+            take_tracking(
+                g,
+                &mut self.native_tracking,
+                &mut self.attachment_snapshots,
+                self.sync_attachments,
+            );
         }
     }
 
@@ -1763,6 +1776,7 @@ impl Hub {
             &mut g,
             &mut self.native_tracking,
             &mut self.attachment_snapshots,
+            self.sync_attachments,
         );
         self.broadcast(&C::PlayerInfoRemove(play::PlayerInfoRemove {
             uuids: vec![g.uuid],
@@ -1957,16 +1971,21 @@ impl Hub {
                 .rev()
                 .map(|idx| g.chunk_queue.remove(idx))
                 .collect();
+            if self.sync_attachments {
+                queue_snapshots(
+                    &mut self.attachment_snapshots,
+                    g.uuid,
+                    seen.iter().map(|&(x, z)| Holder::Chunk([x, z])),
+                );
+            }
             // A far chunk the save doesn't have leaves the queue unsent.
             seen.extend(far.into_iter().rev().filter_map(|(idx, sent)| {
                 let key = g.far_queue.remove(idx);
+                if sent && self.sync_attachments {
+                    g.far_sent_chunks.insert(key);
+                }
                 sent.then_some(key)
             }));
-            queue_snapshots(
-                &mut self.attachment_snapshots,
-                g.uuid,
-                seen.iter().map(|&(x, z)| Holder::Chunk([x, z])),
-            );
             g.sent_chunks.extend(seen);
         }
     }
@@ -1998,8 +2017,19 @@ impl Hub {
             .collect();
         for (x, z) in forget {
             g.sent_chunks.remove(&(x, z));
+            g.far_sent_chunks.remove(&(x, z));
             g.send(&C::ForgetLevelChunk(play::ForgetLevelChunk { pos: [x, z] }));
         }
+        // Far columns have no attachment snapshots. Resend them as near chunks
+        // after they enter the view, once the level has loaded them.
+        g.far_sent_chunks.retain(|&(x, z)| {
+            if tracked(cx, cz, r, x, z) {
+                g.sent_chunks.remove(&(x, z));
+                false
+            } else {
+                true
+            }
+        });
         requeue(&mut g.chunk_queue, &g.sent_chunks, (cx, cz), r);
         if reach > r {
             requeue_ring(&mut g.far_queue, &g.sent_chunks, (cx, cz), r, reach);
@@ -3688,6 +3718,7 @@ impl Hub {
         }
         g.tracker.clear();
         g.sent_chunks.clear();
+        g.far_sent_chunks.clear();
         g.chunk_queue.clear();
         g.far_queue.clear();
         let id = g.entity_id;
@@ -3714,6 +3745,7 @@ impl Hub {
         g.sent_effects.clear();
         g.center = None;
         g.sent_chunks.clear();
+        g.far_sent_chunks.clear();
         g.chunk_queue.clear();
         g.far_queue.clear();
         g.tracker.clear();
@@ -3805,6 +3837,7 @@ impl Hub {
         // respawn only, and covers a level change with its separate `isChangingDimension` term.
         g.tracker.clear();
         g.sent_chunks.clear();
+        g.far_sent_chunks.clear();
         g.chunk_queue.clear();
         g.far_queue.clear();
         g.center = None;

crates/horizon-protocol/src/attachment_sync.rs

@@ -67,7 +67,9 @@ pub fn encode(message: &Message) -> Result<Vec<u8>> {
                 .to_string()
                 .map_err(|_| Error::new("attachment sync: name is not UTF-16"))?;
             if name.encode_utf16().count() > MAX_NAME {
-                return Err(Error::new(format!("attachment sync: name {name} is too long")));
+                return Err(Error::new(format!(
+                    "attachment sync: name {name} is too long"
+                )));
             }
             w.string(&name);
         }

crates/horizon-world/src/attachments.rs

@@ -72,6 +72,18 @@ pub struct AttachmentType<T: 'static> {
     pub sync: bool,
 }
 
+impl<T> AttachmentType<T> {
+    fn validate_policy(&self) -> Result<(), AttachmentError> {
+        if self.copy_on_death && self.codec.is_none() {
+            return Err(AttachmentError::InvalidCopyPolicy);
+        }
+        if self.sync && self.codec.is_none() {
+            return Err(AttachmentError::InvalidSyncPolicy);
+        }
+        Ok(())
+    }
+}
+
 /// One attachment's new state for a client; `None` means removed.
 #[derive(Clone, Debug, PartialEq)]
 pub struct AttachmentSync {
@@ -183,6 +195,13 @@ fn envelope(value: &dyn Value) -> Option<Compound> {
         envelope
     })
 }
+fn raw_value(raw: &Compound) -> Option<&Compound> {
+    match raw.get(&"value".into()) {
+        Some(Tag::Compound(value)) => Some(value),
+        _ => None,
+    }
+}
+
 enum Entry {
     Raw(Compound),
     Decoded(Box<dyn Value>),
@@ -191,10 +210,7 @@ impl Entry {
     /// The encoded value, as clients receive it.
     fn synced_value(&self) -> Option<&Compound> {
         match self {
-            Self::Raw(raw) => match raw.get(&"value".into()) {
-                Some(Tag::Compound(value)) => Some(value),
-                _ => None,
-            },
+            Self::Raw(raw) => raw_value(raw),
             Self::Decoded(value) => value.encoded(),
         }
     }
@@ -303,12 +319,7 @@ impl Attachments {
         &mut self,
         ty: &AttachmentType<T>,
     ) -> Result<Option<T>, AttachmentError> {
-        if ty.copy_on_death && ty.codec.is_none() {
-            return Err(AttachmentError::InvalidCopyPolicy);
-        }
-        if ty.sync && ty.codec.is_none() {
-            return Err(AttachmentError::InvalidSyncPolicy);
-        }
+        ty.validate_policy()?;
         if self
             .raw
             .get(&ty.namespace.into())
@@ -318,9 +329,7 @@ impl Attachments {
         }
         let value = match self.entries.get(&(ty.namespace.into(), ty.name.into())) {
             Some(Entry::Raw(raw)) => {
-                let Some(Tag::Compound(value)) = raw.get(&"value".into()) else {
-                    return Err(AttachmentError::Decode);
-                };
+                let value = raw_value(raw).ok_or(AttachmentError::Decode)?;
                 (ty.codec.as_ref().ok_or(AttachmentError::Decode)?.decode)(value)?
             }
             Some(Entry::Decoded(value)) => {
@@ -357,12 +366,7 @@ impl Attachments {
         ty: &AttachmentType<T>,
         value: T,
     ) -> Result<(), AttachmentError> {
-        if ty.copy_on_death && ty.codec.is_none() {
-            return Err(AttachmentError::InvalidCopyPolicy);
-        }
-        if ty.sync && ty.codec.is_none() {
-            return Err(AttachmentError::InvalidSyncPolicy);
-        }
+        ty.validate_policy()?;
         if self
             .raw
             .get(&ty.namespace.into())
@@ -426,9 +430,7 @@ impl Attachments {
     }
     /// Queue a value to be sent to clients again; nothing for an unsynced type.
     pub fn mark_sync<T: 'static>(&mut self, ty: &AttachmentType<T>) -> Result<(), AttachmentError> {
-        if ty.sync && ty.codec.is_none() {
-            return Err(AttachmentError::InvalidSyncPolicy);
-        }
+        ty.validate_policy()?;
         if ty.sync {
             self.pending.insert((ty.namespace.into(), ty.name.into()));
         }