Fabric API

Unless I'm misunderstanding something about how we're supposed to integrate with the data-attachment-api-v1 for block entities, it seems like using it with (almost) any block entity that has other update-able state won't work. The API mixins to BlockEntity at createNbt, but a lot of the block entities (e.g. beacons and signs) in vanilla don't actually call that when generating update packets (i.e. through world.updateListeners).

In my mod I've attached additional data to signs, but any time world.updateListeners is called for any reason (changed text, applied dye or wax, etc.), my attached data gets wiped client-side when the sign generates its update packet NBT through createComponentlessNbt and sends it to all the clients without any attached data.

From what I can tell all block entities go through BlockEntityUpdateS2CPacket.create(this) at least before diverging in paths, which uses BlockEntity::toInitialChunkDataNbt to generate the NBT. So to me the best way to hook into it would be to mixin to BlockEntityUpdateS2CPacket.create and wrap the BiFunction parameter (BlockEntity::toInitialChunkDataNbt) so we can modify the NBT it spits out, right?

Just to test I implemented this additional mixin in my own mod and everything seems to be working, but I don't know if there are any potential side effects I've overlooked:
https://github.com/Roundaround/mc-fabric-item-signs/blob/1.21.5/src/main/java/me/roundaround/itemsigns/mixin/BlockEntityUpdateS2CPacketMixin.java

EDIT: Made a repro mod:

Here's a basic repro mod of the issue

https://github.com/Roundaround/fabric-example-mod/tree/issue-4638

The important bits are:

The attachment, with persistence and syncing enabled
- https://github.com/Roundaround/fabric-example-mod/blob/3f0dadba5a47f9dc8e5fbe1c1919b6833068cb1c/src/main/java/com/example/ExampleAttachment.java
Setting the attachment
- https://github.com/Roundaround/fabric-example-mod/blob/3f0dadba5a47f9dc8e5fbe1c1919b6833068cb1c/src/main/java/com/example/mixin/AbstractSignBlockMixin.java
Basic mixin that just renders the current attachment value in a typical nametag thing over the sign. Important aspect is that it is reading the attachment client-side
- https://github.com/Roundaround/fabric-example-mod/blob/3f0dadba5a47f9dc8e5fbe1c1919b6833068cb1c/src/client/java/com/example/mixin/client/AbstractSignBlockEntityRendererMixin.java

With this setup, if you place a sign, you should see a red X above it (because no attachment has been set yet).

If you sneak-click the sign, the attachment will be created/set with a value of 1, so the red X should change to a green 1. Continuing to sneak-click it will continue incrementing.

If you then modify the sign's state in some other way (i.e. edit the text, apply dye or wax, etc.), the green number will be reset back to a red X, as the client "forgets" the attached data. This however is client-side only, meaning if you sneak-click the sign again, it will continue incrementing the value and the tag will return back to a green number like expected.

Just had a closer look at this, this fix is not correct as it will now start trying to sync none synced data attachments. The inital syncing of block entity data should be handled here along with the chunk.

If I'm understanding correctly, the solution then might just be to merge data attachments in the read mixin rather than overwrite them, right?

fabric/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/BlockEntityMixin.java

Lines 58 to 64 in 57dfc3e

    
           @Inject( 
        
           		method = "read", 
        
           		at = @At("RETURN") 
        
           ) 
        
           private void readBlockEntityAttachments(NbtCompound nbt, RegistryWrapper.WrapperLookup registryLookup, CallbackInfo ci) { 
        
           	this.fabric_readAttachmentsFromNbt(nbt, registryLookup); 
        
           }

fabric/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java

Lines 123 to 136 in 57dfc3e

    
           @Override 
        
           public void fabric_readAttachmentsFromNbt(NbtCompound nbt, RegistryWrapper.WrapperLookup wrapperLookup) { 
        
           	// Note on player targets: no syncing can happen here as the networkHandler is still null 
        
           	// Instead it is done on player join (see AttachmentSync) 
        
           	this.fabric_dataAttachments = AttachmentSerializingImpl.deserializeAttachmentData(nbt, wrapperLookup); 
        
           	if (this.fabric_shouldTryToSync() && this.fabric_dataAttachments != null) { 
        
           		this.fabric_dataAttachments.forEach((type, value) -> { 
        
           			if (type.isSynced()) { 
        
           				acknowledgeSynced(type, value, wrapperLookup); 
        
           			} 
        
           		}); 
        
           	} 
        
           }

If all changes to data attachments happen outside of this normal Minecraft sync infrastructure anyway (and therefore skip BlockEntity.read entirely), we don't ever need to remove any data attachments in the process of reading from NBT. So instead of entirely clobbering the existing data, we can read if any exist on the NBT and merge them in...I think

EDIT: This is all of course in theory...I don't have anything set up to modify Fabric API source code and test it out within my mod, and I don't actually understand the whole project setup for running game tests and such for Fabric API directly 😅

I don't have any more time today but I'll try to come up with a simple repro mod tomorrow.

In the meantime, you can take a peek at my mod from before my "fix":

https://github.com/Roundaround/mc-fabric-item-signs/tree/4ca388df2ab1e9a71d845e12f8e00c079dd73721

Particularly where I'm setting the attachment:

https://github.com/Roundaround/mc-fabric-item-signs/blob/4ca388df2ab1e9a71d845e12f8e00c079dd73721/src/main/java/me/roundaround/itemsigns/mixin/SignBlockEntityMixin.java#L166-L180

If I just use markDirty it works, but editing the sign text or applying dye/wax causes the clients to get a new update packet without the attached data, so they think it's been wiped. Modifying it to call updateListeners instead makes that data wipe happen immediately on edit.

Just had a closer look at this, this fix is not correct as it will now start trying to sync none synced data attachments. The inital syncing of block entity data should be handled here along with the chunk.

fabric/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/WorldChunkMixin.java

Lines 61 to 68 in 57dfc3e

    
           @Override 
        
           public void fabric_computeInitialSyncChanges(ServerPlayerEntity player, Consumer<AttachmentChange> changeOutput) { 
        
           	super.fabric_computeInitialSyncChanges(player, changeOutput); 
        
           	for (BlockEntity be : this.getBlockEntities().values()) { 
        
           		((AttachmentTargetImpl) be).fabric_computeInitialSyncChanges(player, changeOutput); 
        
           	} 
        
           }

Can you provide steps to reproduce the issue?

Here's a basic repro mod of the issue (I'll also edit it into the original post):

https://github.com/Roundaround/fabric-example-mod/tree/issue-4638

The important bits are:

The attachment, with persistence and syncing enabled
- https://github.com/Roundaround/fabric-example-mod/blob/3f0dadba5a47f9dc8e5fbe1c1919b6833068cb1c/src/main/java/com/example/ExampleAttachment.java
Setting the attachment
- https://github.com/Roundaround/fabric-example-mod/blob/3f0dadba5a47f9dc8e5fbe1c1919b6833068cb1c/src/main/java/com/example/mixin/AbstractSignBlockMixin.java
Basic mixin that just renders the current attachment value in a typical nametag thing over the sign. Important aspect is that it is reading the attachment client-side
- https://github.com/Roundaround/fabric-example-mod/blob/3f0dadba5a47f9dc8e5fbe1c1919b6833068cb1c/src/client/java/com/example/mixin/client/AbstractSignBlockEntityRendererMixin.java

With this setup, if you place a sign, you should see a red X above it (because no attachment has been set yet).

If you sneak-click the sign, the attachment will be created/set with a value of 1, so the red X should change to a green 1. Continuing to sneak-click it will continue incrementing.

If you then modify the sign's state in some other way (i.e. edit the text, apply dye or wax, etc.), the green number will be reset back to a red X, as the client "forgets" the attached data. This however is client-side only, meaning if you sneak-click the sign again, it will continue incrementing the value and the tag will return back to a green number like expected.

	@Inject(
	method = "read",
	at = @At("RETURN")
	)
	private void readBlockEntityAttachments(NbtCompound nbt, RegistryWrapper.WrapperLookup registryLookup, CallbackInfo ci) {
	this.fabric_readAttachmentsFromNbt(nbt, registryLookup);
	}

	@Override
	public void fabric_readAttachmentsFromNbt(NbtCompound nbt, RegistryWrapper.WrapperLookup wrapperLookup) {
	// Note on player targets: no syncing can happen here as the networkHandler is still null
	// Instead it is done on player join (see AttachmentSync)
	this.fabric_dataAttachments = AttachmentSerializingImpl.deserializeAttachmentData(nbt, wrapperLookup);

	if (this.fabric_shouldTryToSync() && this.fabric_dataAttachments != null) {
	this.fabric_dataAttachments.forEach((type, value) -> {
	if (type.isSynced()) {
	acknowledgeSynced(type, value, wrapperLookup);
	}
	});
	}
	}

	@Override
	public void fabric_computeInitialSyncChanges(ServerPlayerEntity player, Consumer<AttachmentChange> changeOutput) {
	super.fabric_computeInitialSyncChanges(player, changeOutput);

	for (BlockEntity be : this.getBlockEntities().values()) {
	((AttachmentTargetImpl) be).fabric_computeInitialSyncChanges(player, changeOutput);
	}
	}

Share to