diff --git a/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/DataAccessorHandler.java b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/DataAccessorHandler.java new file mode 100644 index 0000000000..ebefc3b6a2 --- /dev/null +++ b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/DataAccessorHandler.java @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2016, 2017, 2018, 2019 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.fabric.impl.attachment; + +import java.util.IdentityHashMap; +import java.util.Map; + +import net.minecraft.world.level.storage.ValueInput; + +import net.fabricmc.fabric.api.attachment.v1.AttachmentTarget; +import net.fabricmc.fabric.api.attachment.v1.AttachmentType; + +/** + * Replacement logic to handle applying attachments when using the /data command. + * This applies the changes using the high level APIs, ensuring that the changes are correctly synced to the client. + */ +public class DataAccessorHandler { + public static final ScopedValue APPLYING_DATA_CHANGE = ScopedValue.newInstance(); + + public static void applyDataChanges(AttachmentTarget target, ValueInput data, Runnable applyData) { + AttachmentTargetImpl targetImpl = (AttachmentTargetImpl) target; + + Map, ?> oldAttachments = targetImpl.fabric_getAttachments(); + ScopedValue.where(APPLYING_DATA_CHANGE, null).run(applyData); + + if (oldAttachments != targetImpl.fabric_getAttachments()) { + throw new AssertionError("Attachment data changed during data change application."); + } + + IdentityHashMap, Object> newAttachments = AttachmentSerializingImpl.deserializeAttachmentData(data); + + if (oldAttachments == null && newAttachments == null) { + // No attachments before or after, nothing to do + return; + } else if (oldAttachments != null && (newAttachments == null || newAttachments.isEmpty())) { + // Clear all attachments - copy keys to avoid ConcurrentModificationException + oldAttachments.keySet().stream() + .filter(AttachmentType::isPersistent) + .toList() + .forEach(target::removeAttached); + return; + } + + // Update the new attachments + newAttachments.forEach((attachmentType, o) -> target.setAttached((AttachmentType) attachmentType, o)); + + // Remove all of the removed attachments - copy keys to avoid ConcurrentModificationException + if (oldAttachments != null) { + oldAttachments.keySet().stream() + .filter(AttachmentType::isPersistent) + .filter(attachmentType -> !newAttachments.containsKey(attachmentType)) + .toList() + .forEach(target::removeAttached); + } + } +} diff --git a/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java index 1fdc8fb9bb..e39e6e2330 100644 --- a/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java +++ b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java @@ -46,6 +46,7 @@ import net.fabricmc.fabric.impl.attachment.AttachmentSerializingImpl; import net.fabricmc.fabric.impl.attachment.AttachmentTargetImpl; import net.fabricmc.fabric.impl.attachment.AttachmentTypeImpl; +import net.fabricmc.fabric.impl.attachment.DataAccessorHandler; import net.fabricmc.fabric.impl.attachment.GlobalAttachmentsImpl; import net.fabricmc.fabric.impl.attachment.sync.AttachmentChange; import net.fabricmc.fabric.impl.attachment.sync.AttachmentSync; @@ -137,6 +138,11 @@ public void fabric_writeAttachmentsToNbt(ValueOutput output) { @Override public void fabric_readAttachmentsFromNbt(ValueInput input) { + if (DataAccessorHandler.APPLYING_DATA_CHANGE.isBound()) { + // DataAccessorHandler handles applying data changes separately. + return; + } + // Note on player targets: no syncing can happen here as the networkHandler is still null // Instead it is done on player join (see AttachmentSync) IdentityHashMap, Object> fromNbt = AttachmentSerializingImpl.deserializeAttachmentData(input); diff --git a/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/BlockDataAccessorMixin.java b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/BlockDataAccessorMixin.java new file mode 100644 index 0000000000..79dac0760e --- /dev/null +++ b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/BlockDataAccessorMixin.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2016, 2017, 2018, 2019 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.fabric.mixin.attachment; + +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; + +import net.minecraft.server.commands.data.BlockDataAccessor; +import net.minecraft.server.commands.data.DataAccessor; +import net.minecraft.world.level.block.entity.BlockEntity; +import net.minecraft.world.level.storage.ValueInput; + +import net.fabricmc.fabric.impl.attachment.DataAccessorHandler; + +@Mixin(BlockDataAccessor.class) +public abstract class BlockDataAccessorMixin implements DataAccessor { + @WrapOperation(method = "setData", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/entity/BlockEntity;loadWithComponents(Lnet/minecraft/world/level/storage/ValueInput;)V")) + public void setData(BlockEntity entity, ValueInput input, Operation original) { + if (entity.getLevel() == null) { + // The block entity is not in a level, just follow the default logic. + original.call(entity, input); + return; + } + + DataAccessorHandler.applyDataChanges(entity, input, () -> original.call(entity, input)); + } +} diff --git a/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/EntityDataAccessorMixin.java b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/EntityDataAccessorMixin.java new file mode 100644 index 0000000000..5c831d0fc3 --- /dev/null +++ b/fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/EntityDataAccessorMixin.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2016, 2017, 2018, 2019 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.fabric.mixin.attachment; + +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; + +import net.minecraft.server.commands.data.DataAccessor; +import net.minecraft.server.commands.data.EntityDataAccessor; +import net.minecraft.world.entity.Entity; +import net.minecraft.world.level.storage.ValueInput; + +import net.fabricmc.fabric.impl.attachment.DataAccessorHandler; + +@Mixin(EntityDataAccessor.class) +public abstract class EntityDataAccessorMixin implements DataAccessor { + @WrapOperation(method = "setData", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/Entity;load(Lnet/minecraft/world/level/storage/ValueInput;)V")) + public void setData(Entity entity, ValueInput input, Operation original) { + if (entity.level() == null) { + // The block entity is not in a level, just follow the default logic. + original.call(entity, input); + return; + } + + DataAccessorHandler.applyDataChanges(entity, input, () -> original.call(entity, input)); + } +} diff --git a/fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json b/fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json index 0d53f8c364..43d9c4a702 100644 --- a/fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json +++ b/fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json @@ -5,11 +5,13 @@ "mixins": [ "AttachmentTargetsMixin", "BannerBlockEntityMixin", + "BlockDataAccessorMixin", "BlockEntityMixin", "ChunkAccessMixin", "ChunkHolderMixin", "ClientboundCustomPayloadPacketAccessor", "DimensionStorageFileFixMixin", + "EntityDataAccessorMixin", "EntityMixin", "ImposterProtoChunkMixin", "LevelChunkMixin", diff --git a/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/CommonAttachmentTests.java b/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/CommonAttachmentTests.java index ebdd78928f..6e5a2690dd 100644 --- a/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/CommonAttachmentTests.java +++ b/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/CommonAttachmentTests.java @@ -359,7 +359,7 @@ void applyToInvalidTarget() throws AttachmentSyncException { * so testing is handled by the testmod instead. */ - private static RegistryAccess mockRA() { + static RegistryAccess mockRA() { RegistryAccess ra = mock(RegistryAccess.class); when(ra.createSerializationContext(any())).thenReturn((RegistryOps) (Object) RegistryOps.create(NbtOps.INSTANCE, ra)); return ra; diff --git a/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/DataAccessorHandlerTests.java b/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/DataAccessorHandlerTests.java new file mode 100644 index 0000000000..c34c760857 --- /dev/null +++ b/fabric-data-attachment-api-v1/src/test/java/net/fabricmc/fabric/test/attachment/DataAccessorHandlerTests.java @@ -0,0 +1,330 @@ +/* + * Copyright (c) 2016, 2017, 2018, 2019 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.fabric.test.attachment; + +import static net.fabricmc.fabric.test.attachment.AttachmentTestMod.MOD_ID; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import com.mojang.serialization.Codec; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import net.minecraft.SharedConstants; +import net.minecraft.core.BlockPos; +import net.minecraft.core.RegistryAccess; +import net.minecraft.nbt.CompoundTag; +import net.minecraft.nbt.TagParser; +import net.minecraft.resources.Identifier; +import net.minecraft.server.Bootstrap; +import net.minecraft.server.commands.data.BlockDataAccessor; +import net.minecraft.server.commands.data.DataAccessor; +import net.minecraft.world.level.Level; +import net.minecraft.world.level.block.Blocks; +import net.minecraft.world.level.block.entity.BlockEntity; +import net.minecraft.world.level.block.entity.BlockEntityType; + +import net.fabricmc.fabric.api.attachment.v1.AttachmentRegistry; +import net.fabricmc.fabric.api.attachment.v1.AttachmentTarget; +import net.fabricmc.fabric.api.attachment.v1.AttachmentType; + +public class DataAccessorHandlerTests { + private static final AttachmentType INT = AttachmentRegistry.createPersistent( + Identifier.fromNamespaceAndPath(MOD_ID, "int"), + Codec.INT + ); + private static final AttachmentType STRING = AttachmentRegistry.createPersistent( + Identifier.fromNamespaceAndPath(MOD_ID, "string"), + Codec.STRING + ); + private static final AttachmentType BOOL = AttachmentRegistry.createPersistent( + Identifier.fromNamespaceAndPath(MOD_ID, "bool"), + Codec.BOOL + ); + private static final AttachmentType NON_PERSISTENT_INT = AttachmentRegistry.create( + Identifier.fromNamespaceAndPath(MOD_ID, "non_persistent_int") + ); + private static final AttachmentType NON_PERSISTENT_STRING = AttachmentRegistry.create( + Identifier.fromNamespaceAndPath(MOD_ID, "non_persistent_string") + ); + + @BeforeAll + static void beforeAll() { + SharedConstants.tryDetectVersion(); + Bootstrap.bootStrap(); + } + + BlockEntity blockEntity; + BlockDataAccessor dataAccessor; + AttachmentTarget.OnAttachedSet callback; + + @BeforeEach + void setUp() { + RegistryAccess ra = CommonAttachmentTests.mockRA(); + this.blockEntity = new BlockEntity(BlockEntityType.CHEST, BlockPos.ZERO, Blocks.CHEST.defaultBlockState()) { }; + Level mockLevel = mock(Level.class); + when(mockLevel.registryAccess()).thenReturn(ra); + blockEntity.setLevel(mockLevel); + this.dataAccessor = new BlockDataAccessor(blockEntity, BlockPos.ZERO); + + this.callback = mock(AttachmentTarget.OnAttachedSet.class); + blockEntity.onAttachedSet(INT).register(callback); + } + + private static void merge(DataAccessor dataAccessor, String nbt) throws CommandSyntaxException { + CompoundTag old = dataAccessor.getData(); + CompoundTag merged = old.copy().merge(TagParser.parseCompoundFully(nbt)); + dataAccessor.setData(merged); + } + + private static void set(DataAccessor dataAccessor, String nbt) throws CommandSyntaxException { + dataAccessor.setData(TagParser.parseCompoundFully(nbt)); + } + + @Test + void setAttachment() throws CommandSyntaxException { + assertFalse(blockEntity.hasAttached(INT)); + merge(dataAccessor, "{\"fabric:attachments\": {\"fabric-data-attachment-api-v1-testmod:int\": 5}}"); + assertEquals(5, blockEntity.getAttached(INT)); + verify(callback).onAttachedSet(null, 5); + } + + @Test + void removeAttachment() throws CommandSyntaxException { + blockEntity.setAttached(INT, 5); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + set(dataAccessor, "{}"); + assertFalse(blockEntity.hasAttached(INT)); + verify(callback).onAttachedSet(5, null); + } + + @Test + void updateAttachment() throws CommandSyntaxException { + blockEntity.setAttached(INT, 1); + reset(callback); + + merge(dataAccessor, "{\"fabric:attachments\": {\"fabric-data-attachment-api-v1-testmod:int\": 5}}"); + assertEquals(5, blockEntity.getAttached(INT)); + verify(callback).onAttachedSet(1, 5); + } + + @Test + void noAttachmentsNoOp() throws CommandSyntaxException { + assertFalse(blockEntity.hasAttached(INT)); + set(dataAccessor, "{}"); + assertFalse(blockEntity.hasAttached(INT)); + verifyNoInteractions(callback); + } + + @Test + void clearAllAttachmentsWithEmptyData() throws CommandSyntaxException { + blockEntity.setAttached(INT, 5); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + set(dataAccessor, "{}"); + assertFalse(blockEntity.hasAttached(INT)); + verify(callback).onAttachedSet(5, null); + } + + @Test + void clearAllAttachmentsWithEmptyAttachments() throws CommandSyntaxException { + blockEntity.setAttached(INT, 5); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + set(dataAccessor, "{\"fabric:attachments\": {}}"); + assertFalse(blockEntity.hasAttached(INT)); + verify(callback).onAttachedSet(5, null); + } + + @Test + void addMultipleAttachments() throws CommandSyntaxException { + assertFalse(blockEntity.hasAttached(INT)); + assertFalse(blockEntity.hasAttached(STRING)); + assertFalse(blockEntity.hasAttached(BOOL)); + + merge(dataAccessor, "{\"fabric:attachments\": {" + + "\"fabric-data-attachment-api-v1-testmod:int\": 42, " + + "\"fabric-data-attachment-api-v1-testmod:string\": \"test\", " + + "\"fabric-data-attachment-api-v1-testmod:bool\": true" + + "}}"); + + assertEquals(42, blockEntity.getAttached(INT)); + assertEquals("test", blockEntity.getAttached(STRING)); + assertEquals(true, blockEntity.getAttached(BOOL)); + } + + @Test + void removeSomeAttachmentsKeepOthers() throws CommandSyntaxException { + blockEntity.setAttached(INT, 10); + blockEntity.setAttached(STRING, "keep"); + blockEntity.setAttached(BOOL, false); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(STRING)); + assertTrue(blockEntity.hasAttached(BOOL)); + + // Only keep STRING attachment + set(dataAccessor, "{\"fabric:attachments\": {\"fabric-data-attachment-api-v1-testmod:string\": \"keep\"}}"); + + assertFalse(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(STRING)); + assertFalse(blockEntity.hasAttached(BOOL)); + assertEquals("keep", blockEntity.getAttached(STRING)); + verify(callback).onAttachedSet(10, null); + } + + @Test + void mixedUpdateAddRemoveUpdate() throws CommandSyntaxException { + blockEntity.setAttached(INT, 1); + blockEntity.setAttached(STRING, "old"); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(STRING)); + assertFalse(blockEntity.hasAttached(BOOL)); + + // Update INT, remove STRING, add BOOL + set(dataAccessor, "{\"fabric:attachments\": {" + + "\"fabric-data-attachment-api-v1-testmod:int\": 999, " + + "\"fabric-data-attachment-api-v1-testmod:bool\": true" + + "}}"); + + assertEquals(999, blockEntity.getAttached(INT)); + assertFalse(blockEntity.hasAttached(STRING)); + assertEquals(true, blockEntity.getAttached(BOOL)); + verify(callback).onAttachedSet(1, 999); + } + + @Test + void nonPersistentAttachmentsKeptWhenClearingWithEmptyData() throws CommandSyntaxException { + blockEntity.setAttached(INT, 5); + blockEntity.setAttached(NON_PERSISTENT_INT, 100); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + + // Clear all data - persistent attachments should be removed, non-persistent kept + set(dataAccessor, "{}"); + + assertFalse(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertEquals(100, blockEntity.getAttached(NON_PERSISTENT_INT)); + verify(callback).onAttachedSet(5, null); + } + + @Test + void nonPersistentAttachmentsKeptWhenClearingWithEmptyAttachments() throws CommandSyntaxException { + blockEntity.setAttached(INT, 5); + blockEntity.setAttached(NON_PERSISTENT_STRING, "keep me"); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + + // Clear all attachments - persistent should be removed, non-persistent kept + set(dataAccessor, "{\"fabric:attachments\": {}}"); + + assertFalse(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + assertEquals("keep me", blockEntity.getAttached(NON_PERSISTENT_STRING)); + verify(callback).onAttachedSet(5, null); + } + + @Test + void nonPersistentAttachmentsKeptWhenSettingPersistentOnes() throws CommandSyntaxException { + blockEntity.setAttached(INT, 1); + blockEntity.setAttached(NON_PERSISTENT_INT, 200); + blockEntity.setAttached(NON_PERSISTENT_STRING, "transient"); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + + // Update only persistent attachment INT - non-persistent should remain + set(dataAccessor, "{\"fabric:attachments\": {\"fabric-data-attachment-api-v1-testmod:int\": 42}}"); + + assertEquals(42, blockEntity.getAttached(INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertEquals(200, blockEntity.getAttached(NON_PERSISTENT_INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + assertEquals("transient", blockEntity.getAttached(NON_PERSISTENT_STRING)); + verify(callback).onAttachedSet(1, 42); + } + + @Test + void mixedPersistentAndNonPersistentRemoval() throws CommandSyntaxException { + blockEntity.setAttached(INT, 10); + blockEntity.setAttached(STRING, "remove"); + blockEntity.setAttached(NON_PERSISTENT_INT, 300); + blockEntity.setAttached(NON_PERSISTENT_STRING, "keep"); + reset(callback); + + assertTrue(blockEntity.hasAttached(INT)); + assertTrue(blockEntity.hasAttached(STRING)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + + // Only keep INT with new value - STRING should be removed, non-persistent should be kept + set(dataAccessor, "{\"fabric:attachments\": {\"fabric-data-attachment-api-v1-testmod:int\": 15}}"); + + assertTrue(blockEntity.hasAttached(INT)); + assertEquals(15, blockEntity.getAttached(INT)); + assertFalse(blockEntity.hasAttached(STRING)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertEquals(300, blockEntity.getAttached(NON_PERSISTENT_INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + assertEquals("keep", blockEntity.getAttached(NON_PERSISTENT_STRING)); + verify(callback).onAttachedSet(10, 15); + } + + @Test + void nonPersistentOnlyUnaffectedByDataCommands() throws CommandSyntaxException { + blockEntity.setAttached(NON_PERSISTENT_INT, 500); + blockEntity.setAttached(NON_PERSISTENT_STRING, "unchanged"); + + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + + // Add persistent attachments via data command - non-persistent should remain + merge(dataAccessor, "{\"fabric:attachments\": {" + + "\"fabric-data-attachment-api-v1-testmod:int\": 7, " + + "\"fabric-data-attachment-api-v1-testmod:string\": \"new\"" + + "}}"); + + assertEquals(7, blockEntity.getAttached(INT)); + assertEquals("new", blockEntity.getAttached(STRING)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_INT)); + assertEquals(500, blockEntity.getAttached(NON_PERSISTENT_INT)); + assertTrue(blockEntity.hasAttached(NON_PERSISTENT_STRING)); + assertEquals("unchanged", blockEntity.getAttached(NON_PERSISTENT_STRING)); + } +} diff --git a/fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java b/fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java index 13eeb9dec0..33d4b0e93f 100644 --- a/fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java +++ b/fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java @@ -33,6 +33,7 @@ import net.minecraft.util.ExtraCodecs; import net.minecraft.util.RandomSource; import net.minecraft.world.InteractionResult; +import net.minecraft.world.entity.animal.chicken.Chicken; import net.minecraft.world.item.ItemStack; import net.minecraft.world.item.Items; import net.minecraft.world.level.block.entity.BlockEntity; @@ -147,6 +148,10 @@ public void onInitialize() { entity.hurtServer(level, level.damageSources().generic(), 1); } }); + + if (entity instanceof Chicken) { + entity.setAttached(SYNCED_ITEM, new ItemStack(Items.EGG, 6)); + } }); } } diff --git a/fabric-data-attachment-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/attachment/client/gametest/SyncGametest.java b/fabric-data-attachment-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/attachment/client/gametest/SyncGametest.java index e87dac12d3..c300528ad4 100644 --- a/fabric-data-attachment-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/attachment/client/gametest/SyncGametest.java +++ b/fabric-data-attachment-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/attachment/client/gametest/SyncGametest.java @@ -25,12 +25,14 @@ import net.minecraft.client.multiplayer.ClientLevel; import net.minecraft.core.BlockPos; +import net.minecraft.network.chat.Component; import net.minecraft.server.MinecraftServer; import net.minecraft.server.level.ServerLevel; import net.minecraft.server.level.ServerPlayer; import net.minecraft.world.entity.Entity; import net.minecraft.world.entity.EntityType; import net.minecraft.world.entity.npc.villager.Villager; +import net.minecraft.world.item.ItemStack; import net.minecraft.world.item.Items; import net.minecraft.world.level.Level; import net.minecraft.world.level.block.Blocks; @@ -109,11 +111,13 @@ public void runTest(ClientGameTestContext context) { var villager = new Villager(EntityType.VILLAGER, level); villager.setNoAi(true); villager.setInvulnerable(true); + villager.setCustomName(Component.literal("TestVillager")); state.villagerId = villager.getUUID(); level.addFreshEntity(villager); setSyncedWithAll(villager); set(villager, AttachmentTestMod.SYNCED_WITH_TARGET); villager.setAttached(AttachmentTestMod.SYNCED_LARGE, AttachmentTestMod.LARGE_DATA); + villager.setAttached(AttachmentTestMod.SYNCED_ITEM, new ItemStack(Items.EGG)); LevelChunk originChunk = level.getChunk(0, 0); setSyncedWithAll(originChunk); @@ -178,6 +182,19 @@ public void runTest(ClientGameTestContext context) { client.options.renderDistance().set(12); }); + // Test modifying attachments using the data command, and that the changes are synced to the client. + serverContext.runCommand("data modify entity @n[name=\"TestVillager\"] \"fabric:attachments\".\"fabric-data-attachment-api-v1-testmod:synced_item\".id set value \"minecraft:diamond\""); + context.waitTick(); + context.runOnClient(client -> { + ClientLevel level = Objects.requireNonNull(client.level); + Entity villager = level.getEntity(state.villagerId); + ItemStack syncedItem = villager.getAttached(AttachmentTestMod.SYNCED_ITEM); + + if (syncedItem.getItem() != Items.DIAMOND) { + throw new AssertionError("Unexpected synced item: %s".formatted(syncedItem.getItem())); + } + }); + LOGGER.info("Setting up second phase"); // now teleport to nether, on roof to avoid suffocation when switching to survival serverContext.runCommand("execute in minecraft:the_nether run tp @p ~ 128 ~");