1.21.5 porting fixes (#4527)

* Fix behavior change in `SingleVariantStorage` serialization

25w09a port added `orElseThrow()` call in `readNbt`, changing the behavior for
missing `variant` field. This previously used `fallback` (or, if the `codec` was
somehow a unit codec/could accept empty NBT, successfully decoded), but the change
made it throw. This makes it use the fallback in both cases.
Given that it is unlikely for a variant to use a unit codec, the remaining change is
deemed acceptable.

* Fix handling of incorrect NBT type in `AttachmentSerializingImpl`

* fix typo

* Remove broken wandering trader trade registration code

* Fix typo

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/AttachmentSerializingImpl.java

@@ -18,6 +18,7 @@ package net.fabricmc.fabric.impl.attachment;
 
 import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.Optional;
 
 import com.mojang.serialization.Codec;
 import org.jetbrains.annotations.Nullable;
@@ -65,9 +66,11 @@ public class AttachmentSerializingImpl {
 
 	@Nullable
 	public static IdentityHashMap<AttachmentType<?>, Object> deserializeAttachmentData(NbtCompound nbt, RegistryWrapper.WrapperLookup wrapperLookup) {
-		if (nbt.contains(AttachmentTarget.NBT_ATTACHMENT_KEY)) {
+		Optional<NbtCompound> optional = nbt.getCompound(AttachmentTarget.NBT_ATTACHMENT_KEY);
+
+		if (optional.isPresent()) {
 			var attachments = new IdentityHashMap<AttachmentType<?>, Object>();
-			NbtCompound compound = nbt.getCompound(AttachmentTarget.NBT_ATTACHMENT_KEY).orElseThrow();
+			NbtCompound compound = optional.get();
 
 			for (String key : compound.getKeys()) {
 				AttachmentType<?> type = AttachmentRegistryImpl.get(Identifier.of(key));

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

@@ -54,7 +54,7 @@ abstract class WorldChunkMixin extends AttachmentTargetsMixin implements Attachm
 	public abstract Map<BlockPos, BlockEntity> getBlockEntities();
 
 	@Inject(method = "<init>(Lnet/minecraft/server/world/ServerWorld;Lnet/minecraft/world/chunk/ProtoChunk;Lnet/minecraft/world/chunk/WorldChunk$EntityLoader;)V", at = @At("TAIL"))
-	private void transferProtoChunkAttachement(ServerWorld world, ProtoChunk protoChunk, WorldChunk.EntityLoader entityLoader, CallbackInfo ci) {
+	private void transferProtoChunkAttachment(ServerWorld world, ProtoChunk protoChunk, WorldChunk.EntityLoader entityLoader, CallbackInfo ci) {
 		AttachmentTargetImpl.transfer(protoChunk, this, false);
 	}
 

fabric-gametest-api-v1/src/main/java/net/fabricmc/fabric/api/gametest/v1/GameTest.java

@@ -68,7 +68,7 @@ public @interface GameTest {
 	BlockRotation rotation() default BlockRotation.NONE;
 
 	/**
-	 * When set the test must be ran manually.
+	 * When set the test must be run manually.
 	 */
 	boolean manualOnly() default false;
 

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/trade/TradeOfferHelper.java

@@ -37,7 +37,7 @@ public final class TradeOfferHelper {
 	 * Registers trade offer factories for use by villagers.
 	 * This adds the same trade offers to current and rebalanced trades.
 	 * To add separate offers for the rebalanced trade experiment, use
-	 * {@link #registerVillagerOffers(VillagerProfession, int, VillagerOffersAdder)}.
+	 * {@link #registerVillagerOffers(RegistryKey, int, VillagerOffersAdder)}.
 	 *
 	 * <p>Below is an example, of registering a trade offer factory to be added a blacksmith with a profession level of 3:
 	 * <blockquote><pre>
@@ -81,27 +81,10 @@ public final class TradeOfferHelper {
 
 	/**
 	 * Registers trade offer factories for use by wandering trades.
-	 * This does not add offers for the rebalanced trade experiment.
-	 * To add rebalanced trades, use {@link #registerRebalancedWanderingTraderOffers}.
-	 *
-	 * @param level the level the trades
-	 * @param factory a consumer to provide the factories
-	 */
-	public static void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) {
-		TradeOfferInternals.registerWanderingTraderOffers(level, factory);
-	}
-
-	/**
-	 * Registers trade offer factories for use by wandering trades.
-	 * This only adds offers for the rebalanced trade experiment.
-	 * To add regular trades, use {@link #registerWanderingTraderOffers(int, Consumer)}.
-	 *
-	 * <p><strong>Experimental feature</strong>. This API may receive changes as necessary to adapt to further experiment changes.
 	 *
 	 * @param factory a consumer to add trade offers
 	 */
-	@ApiStatus.Experimental
-	public static synchronized void registerRebalancedWanderingTraderOffers(Consumer<WanderingTraderOffersBuilder> factory) {
+	public static synchronized void registerWanderingTraderOffers(Consumer<WanderingTraderOffersBuilder> factory) {
 		factory.accept(new TradeOfferInternals.WanderingTraderOffersBuilderImpl());
 	}
 
@@ -114,14 +97,11 @@ public final class TradeOfferHelper {
 	}
 
 	/**
-	 * A builder for rebalanced wandering trader offers.
+	 * A builder for wandering trader offers.
 	 *
-	 * <p><strong>Experimental feature</strong>. This API may receive changes as necessary to adapt to further experiment changes.
-	 *
-	 * @see #registerRebalancedWanderingTraderOffers(Consumer)
+	 * @see #registerWanderingTraderOffers(Consumer)
 	 */
 	@ApiStatus.NonExtendable
-	@ApiStatus.Experimental
 	public interface WanderingTraderOffersBuilder {
 		/**
 		 * The pool ID for the "buy items" pool.

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/impl/object/builder/TradeOfferInternals.java

@@ -29,8 +29,6 @@ import it.unimi.dsi.fastutil.objects.Object2IntMap;
 import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.tuple.Pair;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import net.minecraft.registry.RegistryKey;
 import net.minecraft.util.Identifier;
@@ -41,8 +39,6 @@ import net.minecraft.village.VillagerProfession;
 import net.fabricmc.fabric.api.object.builder.v1.trade.TradeOfferHelper;
 
 public final class TradeOfferInternals {
-	private static final Logger LOGGER = LoggerFactory.getLogger("fabric-object-builder-api-v1");
-
 	private TradeOfferInternals() {
 	}
 
@@ -71,29 +67,6 @@ public final class TradeOfferInternals {
 		registerOffers(TradeOffers.REBALANCED_PROFESSION_TO_LEVELED_TRADE.computeIfAbsent(profession, key -> new Int2ObjectOpenHashMap<>()), level, trades -> factory.onRegister(trades, true));
 	}
 
-	public static synchronized void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) {
-		final List<TradeOffers.Factory> list = new ArrayList<>();
-		factory.accept(list);
-		WanderingTraderOffersBuilderImpl.initWanderingTraderTrades();
-
-		Int2ObjectMap<TradeOffers.Factory[]> tradeMap = new Int2ObjectOpenHashMap<>();
-
-		TradeOffers.WANDERING_TRADER_TRADES.forEach(pair -> {
-			tradeMap.put(pair.getRight(), pair.getLeft());
-		});
-
-		registerOffers(tradeMap, level, factory);
-
-		List<Pair<TradeOffers.Factory[], Integer>> tradeList = new ArrayList<>();
-
-		tradeMap.forEach((key, value) -> {
-			tradeList.add(Pair.of(value, key));
-		});
-
-		TradeOffers.WANDERING_TRADER_TRADES = tradeList;
-	}
-
-	// Shared code to register offers for both villagers and wandering traders.
 	private static void registerOffers(Int2ObjectMap<TradeOffers.Factory[]> leveledTradeMap, int level, Consumer<List<TradeOffers.Factory>> factory) {
 		final List<TradeOffers.Factory> list = new ArrayList<>();
 		factory.accept(list);

fabric-object-builder-api-v1/src/testmod/java/net/fabricmc/fabric/test/object/builder/VillagerTypeTest1.java

@@ -61,11 +61,7 @@ public class VillagerTypeTest1 implements ModInitializer {
 			factories.add(new SimpleTradeFactory(new TradeOffer(new TradedItem(Items.GOLD_INGOT, 3), Optional.of(new TradedItem(scrap, 4)), new ItemStack(Items.NETHERITE_INGOT), 2, 6, 0.15F)));
 		});
 
-		TradeOfferHelper.registerWanderingTraderOffers(1, factories -> {
-			factories.add(new SimpleTradeFactory(new TradeOffer(new TradedItem(Items.GOLD_INGOT, 3), Optional.of(new TradedItem(Items.NETHERITE_SCRAP, 4)), new ItemStack(Items.NETHERITE_INGOT), 2, 6, 0.35F)));
-		});
-
-		TradeOfferHelper.registerRebalancedWanderingTraderOffers(builder -> {
+		TradeOfferHelper.registerWanderingTraderOffers(builder -> {
 			builder.pool(
 					FOOD_POOL_ID,
 					5,

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/base/SingleVariantStorage.java

@@ -19,9 +19,6 @@ package net.fabricmc.fabric.api.transfer.v1.storage.base;
 import java.util.function.Supplier;
 
 import com.mojang.serialization.Codec;
-import com.mojang.serialization.DataResult;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import net.minecraft.nbt.NbtCompound;
 import net.minecraft.nbt.NbtElement;
@@ -47,8 +44,6 @@ import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
  * @see net.fabricmc.fabric.api.transfer.v1.item.base.SingleItemStorage SingleItemStorage for item variants.
  */
 public abstract class SingleVariantStorage<T extends TransferVariant<?>> extends SnapshotParticipant<ResourceAmount<T>> implements SingleSlotStorage<T> {
-	private static final Logger LOGGER = LoggerFactory.getLogger("fabric-transfer-api-v1/variant-storage");
-
 	public T variant = getBlankVariant();
 	public long amount = 0;
 
@@ -174,16 +169,8 @@ public abstract class SingleVariantStorage<T extends TransferVariant<?>> extends
 	 */
 	public static <T extends TransferVariant<?>> void readNbt(SingleVariantStorage<T> storage, Codec<T> codec, Supplier<T> fallback, NbtCompound nbt, RegistryWrapper.WrapperLookup wrapperLookup) {
 		final RegistryOps<NbtElement> ops = wrapperLookup.getOps(NbtOps.INSTANCE);
-		final DataResult<T> result = codec.parse(ops, nbt.getCompound("variant").orElseThrow());
-
-		if (result.error().isPresent()) {
-			LOGGER.debug("Failed to load an ItemVariant from NBT: {}", result.error().get());
-			storage.variant = fallback.get();
-		} else {
-			storage.variant = result.result().get();
-		}
-
-		storage.amount = nbt.getLong("amount").orElseThrow();
+		storage.variant = nbt.get("variant", codec, ops).orElseGet(fallback);
+		storage.amount = nbt.getLong("amount", 0L);
 	}
 
 	/**
@@ -197,7 +184,7 @@ public abstract class SingleVariantStorage<T extends TransferVariant<?>> extends
 	 */
 	public static <T extends TransferVariant<?>> void writeNbt(SingleVariantStorage<T> storage, Codec<T> codec, NbtCompound nbt, RegistryWrapper.WrapperLookup wrapperLookup) {
 		final RegistryOps<NbtElement> ops = wrapperLookup.getOps(NbtOps.INSTANCE);
-		nbt.put("variant", codec.encode(storage.variant, ops, nbt).getOrThrow(RuntimeException::new));
+		nbt.put("variant", codec, ops, storage.variant);
 		nbt.putLong("amount", storage.amount);
 	}
 }