Skip to content

Support modifying data attachments using the /data command#5280

Open
modmuss50 wants to merge 4 commits intoFabricMC:26.1from
modmuss50:data-command-attachments
Open

Support modifying data attachments using the /data command#5280
modmuss50 wants to merge 4 commits intoFabricMC:26.1from
modmuss50:data-command-attachments

Conversation

@modmuss50
Copy link
Copy Markdown
Member

This PR add a seperate set of logic for handling data attachment updates when using the /data command. This ensures that the correct events are fired, and the changes are synced to the client if required. It does so by using the higher level API's for updating data attachments as opposed to overwriting the list of attachments without progagating the updates as required.

Fixes #5275

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds dedicated handling for /data-driven updates to Fabric data attachments so attachment callbacks and sync behavior stay consistent (fixing desync reported in #5275).

Changes:

  • Wraps vanilla EntityDataAccessor#setData and BlockDataAccessor#setData to apply attachment changes through Fabric’s high-level attachment APIs.
  • Skips the normal “read attachments from NBT” path while /data is being applied, and applies diffs via a new DataAccessorHandler.
  • Adds unit tests plus a client gametest to validate /data modify updates sync to clients.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/DataAccessorHandler.java New helper that diffs and applies attachment changes during /data operations.
fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/BlockDataAccessorMixin.java Wraps block /data application to route attachment updates through DataAccessorHandler.
fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/EntityDataAccessorMixin.java Wraps entity /data application to route attachment updates through DataAccessorHandler.
fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java Avoids applying attachment NBT directly while a /data change is in progress.
fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json Registers the new accessor mixins.
fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/DataAccessorHandlerTests.java New unit tests validating attachment add/update/remove behavior via DataAccessor set/merge.
fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/CommonAttachmentTests.java Exposes mockRA() for reuse by new unit tests.
fabric-data-attachment-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/attachment/client/gametest/SyncGametest.java Extends client gametest to verify /data modify updates a synced attachment and reaches the client.
fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java Adds chicken initialization behavior for a synced item attachment in the testmod.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/data modify Causes Syncable Data Attachments to Desync

3 participants