Fix attachment sync review findings

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

@@ -31,8 +31,8 @@ fn recipients(hub: &Hub, holder: Holder) -> impl Iterator<Item = &Guest> {
 
 impl Game<'_> {
     /// The messages that give a player `holder`'s synced attachments whole: its
-    /// own, and for a chunk those of its block entities; a holder with none
-    /// gives no message.
+    /// own, and for a chunk those of its block entities. A player holder with
+    /// none still gives one, to drop what the client holds.
     fn attachment_snapshot(&mut self, holder: Holder) -> Vec<Message> {
         let changes = match holder {
             // Read in place: a chunk lent to a mod is copied and saved.
@@ -67,7 +67,9 @@ impl Game<'_> {
         }
         holders
             .into_iter()
-            .filter(|(_, changes)| !changes.is_empty())
+            .filter(|(holder, changes)| {
+                !changes.is_empty() || matches!(holder, Holder::Player(_))
+            })
             .map(|(holder, changes)| Message {
                 holder,
                 replace: true,
@@ -82,6 +84,9 @@ 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/remote.rs

@@ -3223,7 +3223,9 @@ impl Game<'_> {
             C::RemoveEntities(p) => {
                 for id in p.ids {
                     if let Some(r) = self.remote.as_mut() {
-                        r.level_players.remove(&id);
+                        if let Some(uuid) = r.level_players.remove(&id) {
+                            r.attachments.remove(&Holder::Player(uuid));
+                        }
                         r.level_player_names.remove(&id);
                         r.level_player_textures.remove(&id);
                         r.attachments.remove(&Holder::Entity(id as u32));

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

@@ -503,7 +503,7 @@ impl Guest {
         self.placed && self.brand == "horizon"
     }
     /// A holder's synced attachments, if the guest reads them; a message too big
-    /// for one payload goes in halves.
+    /// for one payload is not sent.
     pub fn send_attachment_sync(&self, message: &attachment_sync::Message) {
         if !self.reads_attachment_sync() {
             return;
@@ -513,21 +513,6 @@ impl Guest {
                 channel: attachment_sync::CHANNEL.into(),
                 data,
             })),
-            // One change is the unit that cannot be sent; the others still can.
-            Err(e) if message.changes.len() > 1 => {
-                let (first, rest) = message.changes.split_at(message.changes.len() / 2);
-                self.send_attachment_sync(&attachment_sync::Message {
-                    holder: message.holder,
-                    replace: message.replace,
-                    changes: first.to_vec(),
-                });
-                self.send_attachment_sync(&attachment_sync::Message {
-                    holder: message.holder,
-                    replace: false,
-                    changes: rest.to_vec(),
-                });
-                log::debug!("Horizon: attachment sync split: {e}");
-            }
             Err(e) => log::debug!("Horizon: attachment sync not sent: {e}"),
         }
     }

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

@@ -224,16 +224,12 @@ pub struct AttachmentSnapshot {
     pub holder: Holder,
 }
 
-/// Queues `holders` as snapshots for `player`; with no mod loaded there is
-/// nothing to sync, so none are queued.
+/// Queues `holders` as snapshots for `player`.
 fn queue_snapshots(
     snapshots: &mut Vec<AttachmentSnapshot>,
     player: u128,
     holders: impl IntoIterator<Item = Holder>,
 ) {
-    if crate::mod_api::mods().is_empty() {
-        return;
-    }
     snapshots.extend(
         holders
             .into_iter()
@@ -1900,17 +1896,16 @@ impl Hub {
                 .rev()
                 .map(|idx| g.chunk_queue.remove(idx))
                 .collect();
-            // Far columns are terrain only: no snapshot.
-            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);
                 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);
         }
     }

crates/horizon-protocol/src/attachment_sync.rs

@@ -36,7 +36,7 @@ pub struct Message {
 
 /// A message's payload; an error for a holder it cannot write, a name past
 /// [`MAX_NAME`], a value that is not an encodable compound, or a payload past
-/// [`MAX_PAYLOAD`] (the sender splits the changes and tries again).
+/// [`MAX_PAYLOAD`].
 pub fn encode(message: &Message) -> Result<Vec<u8>> {
     let mut w = Writer::new();
     match message.holder {

docs/native-mods.md

@@ -2197,13 +2197,12 @@ players who track the holder: guests who were sent a chunk for a chunk or block
 who track an entity for an entity, the player and those tracking them for a player, and
 everyone in the level for the level. A client also gets all of a holder's synced values
 when it starts seeing it: a chunk sent (with its block entities), a level joined or
-re-entered, an entity tracked, and its own player. Far columns get none. With no native
+re-entered, an entity tracked, and its own player. With no native
 mod loaded nothing is sent. The wire form is the codec's compound, so `sync` requires
 a codec.
 
-Only Horizon clients are sent values. A change whose namespace or name is over 256
-characters, whose value cannot be written or that alone takes a message past 1 MiB
-is not sent; the holder's others are, in more messages when they do not fit in one.
+Only Horizon clients are sent values. A message with a namespace or name over 256
+characters, a value that cannot be written or a payload past 1 MiB is not sent.
 
 On a client, `Host::attachments` returns a copy of what the server synced, which `get`
 decodes; `set` and `remove` change only that copy, which is dropped when the chunk,