Update read signature and document

src/main/java/net/neoforged/neoforge/attachment/AttachmentSync.java

@@ -216,8 +216,8 @@ public static void receiveSyncedDataAttachments(AttachmentHolder holder, Registr
                 if (syncHandler == null) {
                     throw new IllegalArgumentException("Received synced attachment type without a sync handler registered: " + NeoForgeRegistries.ATTACHMENT_TYPES.getKey(type));
                 }
-                // TODO: need to be careful that the right holder is passed! (when delegating!)
-                var result = syncHandler.read(holder.getExposedHolder(), buf);
+                var previousValue = holder.attachments == null ? null : holder.attachments.get(type);
+                var result = syncHandler.read(holder.getExposedHolder(), buf, previousValue);
                 if (result == null) {
                     if (holder.attachments != null) {
                         holder.attachments.remove(type);

src/main/java/net/neoforged/neoforge/attachment/AttachmentType.java

@@ -252,7 +252,7 @@ public void write(RegistryFriendlyByteBuf buf, T attachment, boolean initialSync
                 }
 
                 @Override
-                public T read(IAttachmentHolder holder, RegistryFriendlyByteBuf buf) {
+                public T read(IAttachmentHolder holder, RegistryFriendlyByteBuf buf, @Nullable T previousValue) {
                     return streamCodec.decode(buf);
                 }
             });

src/main/java/net/neoforged/neoforge/attachment/IAttachmentHolder.java

@@ -12,7 +12,6 @@
 /**
  * An object that can hold data attachments.
  */
-// TODO: needs a method to force re-sync an attachment
 public interface IAttachmentHolder {
     /**
      * Returns {@code true} if there is any data attachments, {@code false} otherwise.

src/main/java/net/neoforged/neoforge/attachment/IAttachmentSyncHandler.java

@@ -19,13 +19,42 @@
  * {@link IAttachmentHolder#syncData(AttachmentType)} can be called to trigger syncing.
  */
 public interface IAttachmentSyncHandler<T> {
+    /**
+     * Decides whether data should be sent to some player that can see the holder.
+     *
+     * <p>By default, all players that can see the holder are sent the data.
+     * A typical use case for this method is to only send player-specific data to that player.
+     *
+     * <p>The returned value should be consistent for a given holder and player.
+     *
+     * @param holder the holder for the attachment, can be cast if the subtype is known
+     * @param to the player that might receive the data
+     * @return {@code true} to send data to the player, {@code false} otherwise
+     */
     default boolean sendToPlayer(IAttachmentHolder holder, ServerPlayer to) {
         return true;
     }
 
+    /**
+     * Writes attachment data to a buffer.
+     *
+     * <p>If {@code initialSync} is {@code true},
+     * the data should be written in full because the client does not have any previous data.
+     *
+     * <p>If {@code initialSync} is {@code false},
+     * the client already received a previous version of the data.
+     * In this case, this method is only called once for the attachment,
+     * and the resulting data is broadcast to all relevant players.
+     */
     void write(RegistryFriendlyByteBuf buf, T attachment, boolean initialSync);
 
-    // TODO: we could also return void and let the sync handler call .setData(type, xxx). But that means passing the type somehow.
+    /**
+     * Reads attachment data on the client side.
+     *
+     * @param holder the attachment holder, can be cast if the subtype is known
+     * @param previousValue the previous value of the attachment, or {@code null} if there was no previous value
+     * @return the new value of the attachment, or {@code null} if the attachment should be removed
+     */
     @Nullable
-    T read(IAttachmentHolder holder, RegistryFriendlyByteBuf buf);
+    T read(IAttachmentHolder holder, RegistryFriendlyByteBuf buf, @Nullable T previousValue);
 }

tests/src/main/java/net/neoforged/neoforge/oldtest/AttachmentSyncTest.java

@@ -46,7 +46,7 @@ public void write(RegistryFriendlyByteBuf buf, Integer attachment, boolean initi
                         }
 
                         @Override
-                        public @Nullable Integer read(IAttachmentHolder holder, RegistryFriendlyByteBuf buf) {
+                        public Integer read(IAttachmentHolder holder, RegistryFriendlyByteBuf buf, @Nullable Integer previousValue) {
                             return buf.readInt();
                         }
                     })