Improve data attachment port (#3678)

* Bump yarn

* Apply yarn update

* Fix porting of Data Attachment block entity support

* Fix FrameBlockEntity testmod

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

@@ -19,6 +19,7 @@ package net.fabricmc.fabric.mixin.attachment;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 import net.minecraft.block.entity.BlockEntity;
@@ -30,11 +31,11 @@ import net.fabricmc.fabric.impl.attachment.AttachmentTargetImpl;
 @Mixin(BlockEntity.class)
 abstract class BlockEntityMixin implements AttachmentTargetImpl {
 	@Inject(
-			method = "method_17897", // lambda body in BlockEntity#createFromNbt
-			at = @At(value = "INVOKE", target = "Lnet/minecraft/block/entity/BlockEntity;method_58690(Lnet/minecraft/nbt/NbtCompound;Lnet/minecraft/registry/RegistryWrapper$WrapperLookup;)V")
+			method = "read",
+			at = @At("RETURN")
 	)
-	private static void readBlockEntityAttachments(NbtCompound nbt, RegistryWrapper.WrapperLookup wrapperLookup, String string, BlockEntity blockEntity, CallbackInfoReturnable<BlockEntity> cir) {
-		((AttachmentTargetImpl) blockEntity).fabric_readAttachmentsFromNbt(nbt, wrapperLookup);
+	private void readBlockEntityAttachments(NbtCompound nbt, RegistryWrapper.WrapperLookup registryLookup, CallbackInfo ci) {
+		this.fabric_readAttachmentsFromNbt(nbt, registryLookup);
 	}
 
 	@Inject(

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

@@ -1,53 +0,0 @@
-/*
- * 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 java.util.function.BiFunction;
-
-import org.spongepowered.asm.mixin.Mixin;
-import org.spongepowered.asm.mixin.injection.At;
-import org.spongepowered.asm.mixin.injection.ModifyArg;
-
-import net.minecraft.block.entity.BlockEntity;
-import net.minecraft.nbt.NbtCompound;
-import net.minecraft.network.packet.s2c.play.BlockEntityUpdateS2CPacket;
-import net.minecraft.registry.DynamicRegistryManager;
-
-import net.fabricmc.fabric.api.attachment.v1.AttachmentTarget;
-
-@Mixin(BlockEntityUpdateS2CPacket.class)
-public class BlockEntityUpdateS2CPacketMixin {
-	/*
-	 * Some BEs use their NBT data to sync with client. If nothing is done, that would always sync persistent attachments
-	 * with client, which may be undesirable. To prevent this, we hook into create(BlockEntity) so it uses a getter that
-	 * also removes attachments. Manual sync is still possible by using create(BlockEntity, Function).
-	 */
-	@ModifyArg(
-			method = "create(Lnet/minecraft/block/entity/BlockEntity;)Lnet/minecraft/network/packet/s2c/play/BlockEntityUpdateS2CPacket;",
-			at = @At(
-					value = "INVOKE",
-					target = "Lnet/minecraft/network/packet/s2c/play/BlockEntityUpdateS2CPacket;create(Lnet/minecraft/block/entity/BlockEntity;Ljava/util/function/BiFunction;)Lnet/minecraft/network/packet/s2c/play/BlockEntityUpdateS2CPacket;"
-			)
-	)
-	private static BiFunction<BlockEntity, DynamicRegistryManager, NbtCompound> stripPersistentAttachmentData(BiFunction<BlockEntity, DynamicRegistryManager, NbtCompound> getter) {
-		return (be, drm) -> {
-			NbtCompound nbt = getter.apply(be, drm);
-			nbt.remove(AttachmentTarget.NBT_ATTACHMENT_KEY);
-			return nbt;
-		};
-	}
-}

fabric-data-attachment-api-v1/src/main/resources/fabric-data-attachment-api-v1.mixins.json

@@ -5,7 +5,6 @@
   "mixins": [
     "AttachmentTargetsMixin",
     "BlockEntityMixin",
-    "BlockEntityUpdateS2CPacketMixin",
     "ChunkSerializerMixin",
     "EntityMixin",
     "ServerWorldMixin",

fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/gametest/BlockEntityTests.java

@@ -77,6 +77,7 @@ public class BlockEntityTests implements FabricGameTest {
 			NbtCompound nbt = ((BlockEntityUpdateS2CPacket) packet).getNbt();
 
 			if (nbt != null && nbt.contains(AttachmentTarget.NBT_ATTACHMENT_KEY)) {
+				// Note: this is a vanilla bug (it called createNbt, instead of the correct createComponentlessNbt)
 				throw new GameTestException("Packet NBT for " + entry + " had persistent data: " + nbt.asString());
 			}
 		}

fabric-data-generation-api-v1/src/main/java/net/fabricmc/fabric/api/datagen/v1/provider/FabricBlockLootTableProvider.java

@@ -91,7 +91,7 @@ public abstract class FabricBlockLootTableProvider extends BlockLootTableGenerat
 
 			for (Identifier blockId : Registries.BLOCK.getIds()) {
 				if (blockId.getNamespace().equals(output.getModId())) {
-					RegistryKey<LootTable> blockLootTableId = Registries.BLOCK.get(blockId).getLootTableId();
+					RegistryKey<LootTable> blockLootTableId = Registries.BLOCK.get(blockId).getLootTableKey();
 
 					if (blockLootTableId.getValue().getNamespace().equals(output.getModId())) {
 						if (!lootTables.containsKey(blockLootTableId)) {

fabric-loot-api-v2/src/main/java/net/fabricmc/fabric/api/loot/v2/LootTableEvents.java

@@ -68,7 +68,7 @@ public final class LootTableEvents {
 	 * {@code
 	 * LootTableEvents.MODIFY.register((key, tableBuilder, source) -> {
 	 *     // If the loot table is for the cobblestone block and it is not overridden by a user:
-	 *     if (Blocks.COBBLESTONE.getLootTableId() == key && source.isBuiltin()) {
+	 *     if (Blocks.COBBLESTONE.getLootTableKey() == key && source.isBuiltin()) {
 	 *         // Create a new loot pool that will hold the diamonds.
 	 *         LootPool.Builder pool = LootPool.builder()
 	 *             // Add diamonds...

fabric-loot-api-v2/src/testmod/java/net/fabricmc/fabric/test/loot/LootTest.java

@@ -36,7 +36,7 @@ public class LootTest implements ModInitializer {
 		// The LootTable.Builder LootPool.Builder methods here should use
 		// prebuilt entries and pools to test the injected methods.
 		LootTableEvents.REPLACE.register((key, original, source) -> {
-			if (Blocks.BLACK_WOOL.getLootTableId() == key) {
+			if (Blocks.BLACK_WOOL.getLootTableKey() == key) {
 				if (source != LootTableSource.VANILLA) {
 					throw new AssertionError("black wool loot table should have LootTableSource.VANILLA, got " + source);
 				}
@@ -54,7 +54,7 @@ public class LootTest implements ModInitializer {
 
 		// Test that the event is stopped when the loot table is replaced
 		LootTableEvents.REPLACE.register((key, original, source) -> {
-			if (Blocks.BLACK_WOOL.getLootTableId() == key) {
+			if (Blocks.BLACK_WOOL.getLootTableKey() == key) {
 				throw new AssertionError("Event should have been stopped from replaced loot table");
 			}
 
@@ -62,11 +62,11 @@ public class LootTest implements ModInitializer {
 		});
 
 		LootTableEvents.MODIFY.register((key, tableBuilder, source) -> {
-			if (Blocks.BLACK_WOOL.getLootTableId() == key && source != LootTableSource.REPLACED) {
+			if (Blocks.BLACK_WOOL.getLootTableKey() == key && source != LootTableSource.REPLACED) {
 				throw new AssertionError("black wool loot table should have LootTableSource.REPLACED, got " + source);
 			}
 
-			if (Blocks.WHITE_WOOL.getLootTableId() == key) {
+			if (Blocks.WHITE_WOOL.getLootTableKey() == key) {
 				if (source != LootTableSource.VANILLA) {
 					throw new AssertionError("white wool loot table should have LootTableSource.VANILLA, got " + source);
 				}
@@ -75,26 +75,26 @@ public class LootTest implements ModInitializer {
 				LootPool pool = LootPool.builder()
 						.with(ItemEntry.builder(Items.GOLD_INGOT).build())
 						.conditionally(SurvivesExplosionLootCondition.builder().build())
-						.apply(SetNameLootFunction.builder(Text.literal("Gold from White Wool"), SetNameLootFunction.class_9475.CUSTOM_NAME).build())
+						.apply(SetNameLootFunction.builder(Text.literal("Gold from White Wool"), SetNameLootFunction.Target.CUSTOM_NAME).build())
 						.build();
 
 				tableBuilder.pool(pool);
 			}
 
 			// We modify red wool to drop diamonds in the test mod resources.
-			if (Blocks.RED_WOOL.getLootTableId() == key && source != LootTableSource.MOD) {
+			if (Blocks.RED_WOOL.getLootTableKey() == key && source != LootTableSource.MOD) {
 				throw new AssertionError("red wool loot table should have LootTableSource.MOD, got " + source);
 			}
 
 			// Modify yellow wool to drop *either* yellow wool or emeralds by adding
 			// emeralds to the same loot pool.
-			if (Blocks.YELLOW_WOOL.getLootTableId() == key) {
+			if (Blocks.YELLOW_WOOL.getLootTableKey() == key) {
 				tableBuilder.modifyPools(poolBuilder -> poolBuilder.with(ItemEntry.builder(Items.EMERALD)));
 			}
 		});
 
 		LootTableEvents.ALL_LOADED.register((resourceManager, lootRegistry) -> {
-			LootTable blackWoolTable = lootRegistry.get(Blocks.BLACK_WOOL.getLootTableId());
+			LootTable blackWoolTable = lootRegistry.get(Blocks.BLACK_WOOL.getLootTableKey());
 
 			if (blackWoolTable == LootTable.EMPTY) {
 				throw new AssertionError("black wool loot table should not be empty");

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/block/FabricBlockSettings.java

@@ -84,7 +84,7 @@ public class FabricBlockSettings extends AbstractBlock.Settings {
 		// more proper way, this copies all the fields, not just the shallow ones.
 		// Fields are added by field definition order.
 		this.jumpVelocityMultiplier(otherAccessor.getJumpVelocityMultiplier());
-		this.drops(otherAccessor.getLootTableId());
+		this.drops(otherAccessor.getLootTableKey());
 		this.allowsSpawning(otherAccessor.getAllowsSpawningPredicate());
 		this.solidBlock(otherAccessor.getSolidBlockPredicate());
 		this.suffocates(otherAccessor.getSuffocationPredicate());
@@ -409,7 +409,7 @@ public class FabricBlockSettings extends AbstractBlock.Settings {
 
 	@Deprecated
 	public FabricBlockSettings drops(RegistryKey<LootTable> dropTableId) {
-		((AbstractBlockSettingsAccessor) this).setLootTableId(dropTableId);
+		((AbstractBlockSettingsAccessor) this).setLootTableKey(dropTableId);
 		return this;
 	}
 

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/mixin/object/builder/AbstractBlockSettingsAccessor.java

@@ -101,7 +101,7 @@ public interface AbstractBlockSettingsAccessor {
 	Optional<AbstractBlock.Offsetter> getOffsetter();
 
 	@Accessor
-	RegistryKey<LootTable> getLootTableId();
+	RegistryKey<LootTable> getLootTableKey();
 
 	@Accessor
 	boolean getBlockBreakParticles();
@@ -150,7 +150,7 @@ public interface AbstractBlockSettingsAccessor {
 	void setIsAir(boolean isAir);
 
 	@Accessor
-	void setLootTableId(RegistryKey<LootTable> lootTableId);
+	void setLootTableKey(RegistryKey<LootTable> lootTableKey);
 
 	@Accessor
 	void setToolRequired(boolean toolRequired);

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/trackers/vanilla/BlockInitTracker.java

@@ -42,10 +42,10 @@ public final class BlockInitTracker implements RegistryEntryAddedCallback<Block>
 
 	@Override
 	public void onEntryAdded(int rawId, Identifier id, Block object) {
-		// if false, getDropTableId() will generate an invalid drop table ID
+		// if false, getLootTableKey() will generate an invalid loot table key
 		assert id.equals(registry.getId(object));
 
-		object.getLootTableId();
+		object.getLootTableKey();
 	}
 
 	public static void postFreeze() {

fabric-renderer-api-v1/src/testmod/java/net/fabricmc/fabric/test/renderer/FrameBlockEntity.java

@@ -100,6 +100,6 @@ public class FrameBlockEntity extends BlockEntity implements RenderDataBlockEnti
 
 	@Override
 	public NbtCompound toInitialChunkDataNbt(RegistryWrapper.WrapperLookup wrapperLookup) {
-		return this.createNbt(wrapperLookup);
+		return this.createComponentlessNbt(wrapperLookup);
 	}
 }

gradle.properties

@@ -4,7 +4,7 @@ fabric.loom.multiProjectOptimisation=true
 
 version=0.96.13
 minecraft_version=24w13a
-yarn_version=+build.2
+yarn_version=+build.7
 loader_version=0.15.6
 installer_version=0.11.1