TradeOfferHelper: support rebalance experiment

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

@@ -19,6 +19,8 @@
 import java.util.List;
 import java.util.function.Consumer;
 
+import org.apache.commons.lang3.tuple.Pair;
+
 import net.minecraft.village.TradeOffers;
 import net.minecraft.village.VillagerProfession;
 
@@ -29,11 +31,25 @@
  */
 public final class TradeOfferHelper {
 	/**
-	 * Registers trade offer factories for use by villagers.
+	 * Registers trade offer factories for use by villagers. This registers the same trades regardless of
+	 * whether the rebalanced trade experiment is enabled.
+	 *
+	 * @param profession the villager profession to assign the trades to
+	 * @param level the profession level the villager must be to offer the trades
+	 * @param factories a consumer to provide the factories
+	 * @deprecated Use {@link #registerVillagerOffers(VillagerProfession, int, VillagerTradeRegistrationCallback)} instead.
+	 */
+	public static void registerVillagerOffers(VillagerProfession profession, int level, Consumer<List<TradeOffers.Factory>> factories) {
+		TradeOfferInternals.registerVillagerOffers(profession, level, (trades, rebalanced) -> factories.accept(trades));
+	}
+
+	/**
+	 * Registers trade offer factories for use by villagers. This allows mods to register different
+	 * trades depending on whether the trades are for the rebalanced trade experiment.
 	 *
 	 * <p>Below is an example, of registering a trade offer factory to be added a blacksmith with a profession level of 3:
 	 * <blockquote><pre>
-	 * TradeOfferHelper.registerVillagerOffers(VillagerProfession.BLACKSMITH, 3, factories -> {
+	 * TradeOfferHelper.registerVillagerOffers(VillagerProfession.BLACKSMITH, 3, (factories, rebalanced) -> {
 	 * 	factories.add(new CustomTradeFactory(...);
 	 * });
 	 * </pre></blockquote>
@@ -42,20 +58,38 @@ public final class TradeOfferHelper {
 	 * @param level the profession level the villager must be to offer the trades
 	 * @param factories a consumer to provide the factories
 	 */
-	public static void registerVillagerOffers(VillagerProfession profession, int level, Consumer<List<TradeOffers.Factory>> factories) {
+	public static void registerVillagerOffers(VillagerProfession profession, int level, VillagerTradeRegistrationCallback factories) {
 		TradeOfferInternals.registerVillagerOffers(profession, level, factories);
 	}
 
 	/**
 	 * Registers trade offer factories for use by wandering trades.
+	 * If the rebalanced trade experiment is enabled, {@code level} is ignored,
+	 * and a fixed number of randomly chosen trades registered by this method will always appear.
+	 * This number is currently 25%; this is subject to change.
 	 *
-	 * @param level the level the trades
+	 * @param level the level of the trades
 	 * @param factory a consumer to provide the factories
+	 * @deprecated Use {@link #registerWanderingTraderOffers(int, WanderingTraderTradeRegistrationCallback)} instead.
+	 * Given the inherent design incompatibility that needs to be addressed by mod developers, this is deprecated for removal.
 	 */
+	@Deprecated(forRemoval = true)
 	public static void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) {
 		TradeOfferInternals.registerWanderingTraderOffers(level, factory);
 	}
 
+	/**
+	 * Registers trade offer factories for use by wandering trades.
+	 * If the rebalanced trade experiment is enabled, {@code level} is ignored.
+	 * If the experiment is not enabled, the weight is ignored.
+	 *
+	 * @param level the level of the trades
+	 * @param factory a consumer to provide the factories
+	 */
+	public static void registerWanderingTraderOffers(int level, WanderingTraderTradeRegistrationCallback factory) {
+		TradeOfferInternals.registerWanderingTraderOffers(level, factory);
+	}
+
 	/**
 	 * @deprecated This never did anything useful.
 	 */
@@ -66,4 +100,27 @@ public static void refreshOffers() {
 
 	private TradeOfferHelper() {
 	}
+
+	@FunctionalInterface
+	public interface VillagerTradeRegistrationCallback {
+		/**
+		 * Callback to register villager trades.
+		 * @param trades the list to add trades to
+		 * @param rebalanced whether the trades are for the rebalanced trade experiment
+		 */
+		void onRegister(List<TradeOffers.Factory> trades, boolean rebalanced);
+	}
+
+	@FunctionalInterface
+	public interface WanderingTraderTradeRegistrationCallback {
+		/**
+		 * Callback to register weighted wandering trader trades.
+		 *
+		 * <p>A trade offer pool entry is an array of trades, and the number of rolls from the pool.
+		 * If the number of rolls is equal to or above the size of the array, all trades are included.
+		 * @param trades the list to add trade offer pool entries to
+		 * @param rebalanced whether the trades are for the rebalanced trade experiment
+		 */
+		void onRegister(List<Pair<TradeOffers.Factory[], Integer>> trades, boolean rebalanced);
+	}
 }

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

@@ -17,19 +17,26 @@
 package net.fabricmc.fabric.impl.object.builder;
 
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 
 import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
 import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
 import org.apache.commons.lang3.ArrayUtils;
-import org.slf4j.LoggerFactory;
+import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import net.minecraft.util.math.MathHelper;
 import net.minecraft.village.TradeOffers;
 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");
 
@@ -38,19 +45,63 @@ private TradeOfferInternals() {
 
 	// synchronized guards against concurrent modifications - Vanilla does not mutate the underlying arrays (as of 1.16),
 	// so reads will be fine without locking.
-	public static synchronized void registerVillagerOffers(VillagerProfession profession, int level, Consumer<List<TradeOffers.Factory>> factory) {
+	public static synchronized void registerVillagerOffers(VillagerProfession profession, int level, TradeOfferHelper.VillagerTradeRegistrationCallback factory) {
 		Objects.requireNonNull(profession, "VillagerProfession may not be null.");
-		registerOffers(TradeOffers.PROFESSION_TO_LEVELED_TRADE.computeIfAbsent(profession, key -> new Int2ObjectOpenHashMap<>()), level, factory);
+
+		// Make the map modifiable
+		if (!(TradeOffers.REBALANCED_PROFESSION_TO_LEVELED_TRADE instanceof HashMap)) {
+			TradeOffers.REBALANCED_PROFESSION_TO_LEVELED_TRADE = new HashMap<>(TradeOffers.REBALANCED_PROFESSION_TO_LEVELED_TRADE);
+		}
+
+		registerOffers(TradeOffers.REBALANCED_PROFESSION_TO_LEVELED_TRADE.computeIfAbsent(profession, key -> {
+			// Absence of the trade entry in rebalanced map means "check normal trade map".
+			// If we just add an empty map, vanilla trades would not be available if rebalanced trade is used.
+			// Copy the vanilla map here instead. Successive calls modify the copy only.
+			final Int2ObjectMap<TradeOffers.Factory[]> vanillaMap = TradeOffers.PROFESSION_TO_LEVELED_TRADE.get(profession);
+
+			if (vanillaMap != null) {
+				return new Int2ObjectOpenHashMap<>(vanillaMap);
+			}
+
+			// Custom profession; vanilla trades unavailable, so just make a new map.
+			return new Int2ObjectOpenHashMap<>();
+		}), level, factory::onRegister, true); // casting functional interface
+		// This must be done AFTER the rebalanced trade map is changed, to avoid double registration
+		// for the first call (by copying the already-registered map).
+		registerOffers(TradeOffers.PROFESSION_TO_LEVELED_TRADE.computeIfAbsent(profession, key -> new Int2ObjectOpenHashMap<>()), level, factory::onRegister, false);
 	}
 
 	public static synchronized void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) {
-		registerOffers(TradeOffers.WANDERING_TRADER_TRADES, level, factory);
+		registerOffers(TradeOffers.WANDERING_TRADER_TRADES, level, (trades, rebalanced) -> factory.accept(trades), false); // rebalanced arg unused
+
+		// Rebalanced wandering trader offers are not leveled.
+		registerRebalancedWanderingTraderOffers(poolList -> {
+			final List<TradeOffers.Factory> list = new ArrayList<>();
+			factory.accept(list);
+			// The likely intent of the mod is to offer some of the entries, but not all.
+			// This was previously done by entirely random offer; now that fixed-count pool is
+			// used, if we add elements of the list one by one, they would all show up.
+			// Offer 25% (arbitrary number chosen by apple502j) of the registered trades at a time.
+			// Wandering traders are not Amazon.
+			poolList.add(Pair.of(list.toArray(TradeOffers.Factory[]::new), MathHelper.ceil(list.size() / 4.0)));
+		});
+	}
+
+	public static synchronized void registerWanderingTraderOffers(int level, TradeOfferHelper.WanderingTraderTradeRegistrationCallback callback) {
+		registerOffers(TradeOffers.WANDERING_TRADER_TRADES, level, (list, rebalanced) -> {
+			List<Pair<TradeOffers.Factory[], Integer>> trades = new ArrayList<>();
+			callback.onRegister(trades, false);
+			trades.forEach(trade -> list.addAll(Arrays.asList(trade.getLeft())));
+		}, false); // rebalanced arg unused
+
+		// Rebalanced wandering trader offers are not leveled.
+		registerRebalancedWanderingTraderOffers(poolList -> callback.onRegister(poolList, true));
 	}
 
-	// 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) {
+	// Shared code to register offers for both villagers and non-rebalanced wandering traders.
+	private static void registerOffers(Int2ObjectMap<TradeOffers.Factory[]> leveledTradeMap, int level, BiConsumer<List<TradeOffers.Factory>, Boolean> factory, boolean rebalanced) {
 		final List<TradeOffers.Factory> list = new ArrayList<>();
-		factory.accept(list);
+		factory.accept(list, rebalanced);
 
 		final TradeOffers.Factory[] originalEntries = leveledTradeMap.computeIfAbsent(level, key -> new TradeOffers.Factory[0]);
 		final TradeOffers.Factory[] addedEntries = list.toArray(new TradeOffers.Factory[0]);
@@ -59,6 +110,15 @@ private static void registerOffers(Int2ObjectMap<TradeOffers.Factory[]> leveledT
 		leveledTradeMap.put(level, allEntries);
 	}
 
+	private static void registerRebalancedWanderingTraderOffers(Consumer<List<Pair<TradeOffers.Factory[], Integer>>> factory) {
+		// Make the list modifiable
+		if (!(TradeOffers.REBALANCED_WANDERING_TRADER_TRADES instanceof ArrayList)) {
+			TradeOffers.REBALANCED_WANDERING_TRADER_TRADES = new ArrayList<>(TradeOffers.REBALANCED_WANDERING_TRADER_TRADES);
+		}
+
+		factory.accept(TradeOffers.REBALANCED_WANDERING_TRADER_TRADES);
+	}
+
 	public static void printRefreshOffersWarning() {
 		Throwable loggingThrowable = new Throwable();
 		LOGGER.warn("TradeOfferHelper#refreshOffers does not do anything, yet it was called! Stack trace:", loggingThrowable);

fabric-object-builder-api-v1/src/main/resources/fabric-object-builder-api-v1.accesswidener

@@ -5,6 +5,8 @@ accessible    method    net/minecraft/world/poi/PointOfInterestTypes    register
 
 extendable class net/minecraft/block/entity/BlockEntityType$BlockEntityFactory
 accessible class net/minecraft/village/TradeOffers$TypeAwareBuyForOneEmeraldFactory
+mutable field net/minecraft/village/TradeOffers REBALANCED_PROFESSION_TO_LEVELED_TRADE Ljava/util/Map;
+mutable field net/minecraft/village/TradeOffers REBALANCED_WANDERING_TRADER_TRADES Ljava/util/List;
 
 accessible method net/minecraft/entity/SpawnRestriction register (Lnet/minecraft/entity/EntityType;Lnet/minecraft/entity/SpawnRestriction$Location;Lnet/minecraft/world/Heightmap$Type;Lnet/minecraft/entity/SpawnRestriction$SpawnPredicate;)V
 

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

@@ -22,6 +22,7 @@
 import static net.minecraft.server.command.CommandManager.literal;
 
 import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;
+import org.apache.commons.lang3.tuple.Pair;
 
 import net.minecraft.entity.Entity;
 import net.minecraft.entity.passive.WanderingTraderEntity;
@@ -40,12 +41,30 @@
 public class VillagerTypeTest1 implements ModInitializer {
 	@Override
 	public void onInitialize() {
-		TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
-			factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.GOLD_INGOT, 3), new ItemStack(Items.NETHERITE_SCRAP, 4), new ItemStack(Items.NETHERITE_INGOT), 2, 6, 0.15F)));
+		TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, (factories, rebalanced) -> {
+			if (rebalanced) {
+				factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.GOLD_INGOT, 48), new ItemStack(Items.NETHERITE_SCRAP, 64), new ItemStack(Items.NETHERITE_INGOT, 16), 1, 6, 0.15F)));
+			} else {
+				factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.GOLD_INGOT, 3), new ItemStack(Items.NETHERITE_SCRAP, 4), new ItemStack(Items.NETHERITE_INGOT), 2, 6, 0.15F)));
+			}
 		});
 
-		TradeOfferHelper.registerWanderingTraderOffers(1, factories -> {
-			factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.GOLD_INGOT, 3), new ItemStack(Items.NETHERITE_SCRAP, 4), new ItemStack(Items.NETHERITE_INGOT), 2, 6, 0.35F)));
+		TradeOfferHelper.registerWanderingTraderOffers(1, (factories, rebalanced) -> {
+			if (rebalanced) {
+				factories.add(Pair.of(
+						new TradeOffers.Factory[]{
+								new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.GOLD_INGOT, 48), new ItemStack(Items.NETHERITE_SCRAP, 64), new ItemStack(Items.NETHERITE_INGOT, 16), 1, 6, 0.35F))
+						},
+						1
+				));
+			} else {
+				factories.add(Pair.of(
+						new TradeOffers.Factory[]{
+								new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.GOLD_INGOT, 3), new ItemStack(Items.NETHERITE_SCRAP, 4), new ItemStack(Items.NETHERITE_INGOT), 2, 6, 0.35F))
+						},
+						1
+				));
+			}
 		});
 
 		CommandRegistrationCallback.EVENT.register((dispatcher, registryAccess, environment) -> {

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

@@ -16,8 +16,10 @@
 
 package net.fabricmc.fabric.test.object.builder;
 
+import net.minecraft.item.Item;
 import net.minecraft.item.ItemStack;
 import net.minecraft.item.Items;
+import net.minecraft.registry.Registries;
 import net.minecraft.village.TradeOffer;
 import net.minecraft.village.VillagerProfession;
 
@@ -26,9 +28,11 @@
 
 /*
  * Second entrypoint to validate class loading does not break this.
+ * This is used to test deprecated methods, some of which have their own code path.
  */
 public class VillagerTypeTest2 implements ModInitializer {
 	@Override
+	@SuppressWarnings("deprecation")
 	public void onInitialize() {
 		TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
 			factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 5), new ItemStack(Items.NETHERITE_INGOT), 3, 4, 0.15F)));
@@ -48,5 +52,12 @@ public void onInitialize() {
 		TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
 			factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 10), new ItemStack(Items.CHAINMAIL_LEGGINGS), 3, 4, 0.15F)));
 		});
+		TradeOfferHelper.registerWanderingTraderOffers(1, factory -> {
+			for (Item item : Registries.ITEM) {
+				if (item.getFoodComponent() != null) {
+					factory.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.NETHERITE_INGOT, 1), new ItemStack(item, 1), 3, 4, 0.15F)));
+				}
+			}
+		});
 	}
 }