Fix bug with creating custom villager types and TradeOffer utilities.

fabric-object-builder-api-v1/build.gradle

@@ -4,6 +4,8 @@ version = getSubprojectVersion(project, "1.7.0")
 dependencies {
 	compile project(path: ':fabric-api-base', configuration: 'dev')
 	compile project(path: ':fabric-tool-attribute-api-v1', configuration: 'dev')
+
+	testmodCompile project(path: ':fabric-command-api-v1', configuration: 'dev')
 }
 
 minecraft {

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

@@ -0,0 +1,67 @@
+/*
+ * 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.api.object.builder.v1.trade;
+
+import java.util.List;
+import java.util.function.Consumer;
+
+import net.minecraft.village.TradeOffers;
+import net.minecraft.village.VillagerProfession;
+
+import net.fabricmc.fabric.impl.object.builder.TradeOfferInternals;
+
+/**
+ * Utilities to help with registration of trade offers.
+ */
+public final class TradeOfferHelper {
+	/**
+	 * Registers trade offer factories for use by villagers.
+	 *
+	 * <p>Below is an example, of registering a trade off factory to be added a blacksmith with a profession level of 3:
+	 * <blockquote><pre>
+	 * TradeOfferHelper.registerVillagerOffers(VillagerProfession.BLACKSMITH, 3, factories -> {
+	 * 	factories.add(new CustomTradeFactory(...);
+	 * });
+	 * </pre></blockquote>
+	 *
+	 * @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
+	 */
+	public static void registerVillagerOffers(VillagerProfession profession, int level, Consumer<List<TradeOffers.Factory>> factories) {
+		TradeOfferInternals.registerVillagerOffers(profession, level, factories);
+	}
+
+	/**
+	 * Registers trade offer factories for use by wandering trades.
+	 *
+	 * @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);
+	}
+
+	/**
+	 * Refreshes the trade list by resetting the trade lists to vanilla state, and then registering all trade offers again.
+	 *
+	 * <p>This method is geared for use by mods which for example provide data driven villager trades.
+	 */
+	public static void refreshOffers() {
+		TradeOfferInternals.refreshOffers();
+	}
+}

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

@@ -0,0 +1,122 @@
+/*
+ * 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.object.builder;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+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 net.minecraft.village.TradeOffers;
+import net.minecraft.village.VillagerProfession;
+
+import net.fabricmc.fabric.mixin.object.builder.TradeOffersAccessor;
+
+public final class TradeOfferInternals {
+	/**
+	 * A copy of the original trade offers map.
+	 */
+	public static Map<VillagerProfession, Int2ObjectMap<TradeOffers.Factory[]>> DEFAULT_VILLAGER_OFFERS;
+	public static Int2ObjectMap<TradeOffers.Factory[]> DEFAULT_WANDERING_TRADER_OFFERS;
+	private static final Map<VillagerProfession, Int2ObjectMap<TradeOffers.Factory[]>> VILLAGER_TRADE_FACTORIES = new HashMap<>();
+	private static final Int2ObjectMap<TradeOffers.Factory[]> WANDERING_TRADER_FACTORIES = new Int2ObjectOpenHashMap<>();
+	private TradeOfferInternals() {
+	}
+
+	public static void registerVillagerOffers(VillagerProfession profession, int level, Consumer<List<TradeOffers.Factory>> factory) {
+		final List<TradeOffers.Factory> list = new ArrayList<>();
+		factory.accept(list);
+
+		final TradeOffers.Factory[] additionalEntries = list.toArray(new TradeOffers.Factory[0]);
+		final Int2ObjectMap<TradeOffers.Factory[]> professionEntry = VILLAGER_TRADE_FACTORIES.computeIfAbsent(profession, p -> new Int2ObjectOpenHashMap<>());
+
+		final TradeOffers.Factory[] currentEntries = professionEntry.computeIfAbsent(level, l -> new TradeOffers.Factory[0]);
+		final TradeOffers.Factory[] newEntries = ArrayUtils.addAll(additionalEntries, currentEntries);
+		professionEntry.put(level, newEntries);
+
+		// Refresh the trades map
+		TradeOfferInternals.refreshOffers();
+	}
+
+	public static void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) {
+		final List<TradeOffers.Factory> list = new ArrayList<>();
+		factory.accept(list);
+
+		final TradeOffers.Factory[] additionalEntries = list.toArray(new TradeOffers.Factory[0]);
+		final TradeOffers.Factory[] currentEntries = TradeOfferInternals.DEFAULT_WANDERING_TRADER_OFFERS.computeIfAbsent(level, key -> new TradeOffers.Factory[0]);
+
+		// Merge current and new entries
+		final TradeOffers.Factory[] newEntries = ArrayUtils.addAll(additionalEntries, currentEntries);
+		TradeOfferInternals.DEFAULT_WANDERING_TRADER_OFFERS.put(level, newEntries);
+
+		// Refresh the trades map
+		TradeOfferInternals.refreshOffers();
+	}
+
+	public static void refreshOffers() {
+		TradeOfferInternals.refreshVillagerOffers();
+		TradeOfferInternals.refreshWanderingTraderOffers();
+	}
+
+	private static void refreshVillagerOffers() {
+		final HashMap<VillagerProfession, Int2ObjectMap<TradeOffers.Factory[]>> trades = new HashMap<>(TradeOfferInternals.DEFAULT_VILLAGER_OFFERS);
+
+		for (Map.Entry<VillagerProfession, Int2ObjectMap<TradeOffers.Factory[]>> tradeFactoryEntry : TradeOfferInternals.VILLAGER_TRADE_FACTORIES.entrySet()) {
+			// Create an empty map or get all existing profession entries.
+			final Int2ObjectMap<TradeOffers.Factory[]> leveledFactoryMap = trades.computeIfAbsent(tradeFactoryEntry.getKey(), k -> new Int2ObjectOpenHashMap<>());
+			// Get the existing entries
+			final Int2ObjectMap<TradeOffers.Factory[]> value = tradeFactoryEntry.getValue();
+
+			// Iterate through the existing level entries
+			for (int level : value.keySet()) {
+				final TradeOffers.Factory[] factories = value.get(level);
+
+				if (factories != null) {
+					final Int2ObjectMap<TradeOffers.Factory[]> resultMap = trades.computeIfAbsent(tradeFactoryEntry.getKey(), key -> new Int2ObjectOpenHashMap<>());
+					resultMap.put(level, ArrayUtils.addAll(leveledFactoryMap.computeIfAbsent(level, key -> new TradeOffers.Factory[0]), factories));
+				}
+			}
+		}
+
+		// Set the new villager trade map
+		TradeOffersAccessor.setVillagerTradeMap(trades);
+	}
+
+	private static void refreshWanderingTraderOffers() {
+		// Create an empty map that is a clone of the default offers
+		final Int2ObjectMap<TradeOffers.Factory[]> trades = new Int2ObjectOpenHashMap<>(TradeOfferInternals.DEFAULT_WANDERING_TRADER_OFFERS);
+
+		for (int level : TradeOfferInternals.WANDERING_TRADER_FACTORIES.keySet()) {
+			// Get all registered offers and add them to current entries
+			final TradeOffers.Factory[] factories = TradeOfferInternals.WANDERING_TRADER_FACTORIES.get(level);
+			trades.put(level, ArrayUtils.addAll(factories, trades.computeIfAbsent(level, key -> new TradeOffers.Factory[0])));
+		}
+
+		// Set the new wandering trader trade map
+		TradeOffersAccessor.setWanderingTraderTradeMap(trades);
+	}
+
+	static {
+		// Load the trade offers class so the field is set.
+		TradeOffers.PROFESSION_TO_LEVELED_TRADE.getClass();
+	}
+}

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

@@ -0,0 +1,39 @@
+/*
+ * 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.object.builder;
+
+import java.util.Map;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.gen.Accessor;
+
+import net.minecraft.village.TradeOffers;
+import net.minecraft.village.VillagerProfession;
+
+@Mixin(TradeOffers.class)
+public interface TradeOffersAccessor {
+	@Accessor("PROFESSION_TO_LEVELED_TRADE")
+	static void setVillagerTradeMap(Map<VillagerProfession, Int2ObjectMap<TradeOffers.Factory[]>> trades) {
+		throw new AssertionError("This should not happen!");
+	}
+
+	@Accessor("WANDERING_TRADER_TRADES")
+	static void setWanderingTraderTradeMap(Int2ObjectMap<TradeOffers.Factory[]> trades) {
+		throw new AssertionError("This should not happen!");
+	}
+}

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

@@ -0,0 +1,45 @@
+/*
+ * 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.object.builder;
+
+import java.util.Map;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import org.spongepowered.asm.mixin.Final;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+
+import net.minecraft.village.TradeOffers;
+import net.minecraft.village.VillagerProfession;
+
+import net.fabricmc.fabric.impl.object.builder.TradeOfferInternals;
+
+@Mixin(TradeOffers.class)
+public abstract class TradeOffersMixin {
+	@Shadow
+	@Final
+	public static Map<VillagerProfession, Int2ObjectMap<TradeOffers.Factory[]>> PROFESSION_TO_LEVELED_TRADE;
+	@Shadow
+	@Final
+	public static Int2ObjectMap<TradeOffers.Factory[]> WANDERING_TRADER_TRADES;
+
+	static {
+		// Cache the original trade lists
+		TradeOfferInternals.DEFAULT_VILLAGER_OFFERS = PROFESSION_TO_LEVELED_TRADE;
+		TradeOfferInternals.DEFAULT_WANDERING_TRADER_OFFERS = WANDERING_TRADER_TRADES;
+	}
+}

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

@@ -0,0 +1,56 @@
+/*
+ * 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.object.builder;
+
+import java.util.Random;
+import java.util.stream.Stream;
+
+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.Redirect;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
+import org.spongepowered.asm.mixin.injection.callback.LocalCapture;
+
+import net.minecraft.entity.Entity;
+import net.minecraft.item.ItemStack;
+import net.minecraft.util.registry.DefaultedRegistry;
+import net.minecraft.village.TradeOffer;
+
+@Mixin(targets = "net/minecraft/village/TradeOffers$TypeAwareBuyForOneEmeraldFactory")
+public abstract class TypeAwareTradeMixin {
+	/**
+	 * Vanilla will check the "VillagerType -> Item" map in the stream and throw an exception for villager types not specified in the map.
+	 * This breaks any and all custom villager types.
+	 * We want to prevent this default logic so modded villager types will work.
+	 * So we return an empty stream so an exception is never thrown.
+	 */
+	@Redirect(method = "<init>", at = @At(value = "INVOKE", target = "Lnet/minecraft/util/registry/DefaultedRegistry;stream()Ljava/util/stream/Stream;"))
+	private <T> Stream<T> disableVanillaCheck(DefaultedRegistry<T> registry) {
+		return Stream.empty();
+	}
+
+	/**
+	 * To prevent "item" -> "air" trades, if the result of a type aware trade is air, make sure no offer is created.
+	 */
+	@Inject(method = "create", at = @At(value = "NEW", target = "net/minecraft/village/TradeOffer"), locals = LocalCapture.CAPTURE_FAILEXCEPTION, cancellable = true)
+	private void failOnNullItem(Entity entity, Random random, CallbackInfoReturnable<TradeOffer> cir, ItemStack buyingItem) {
+		if (buyingItem.isEmpty()) { // Will return true for an "empty" item stack that had null passed in the ctor
+			cir.setReturnValue(null); // Return null to prevent creation of empty trades
+		}
+	}
+}

fabric-object-builder-api-v1/src/main/resources/fabric-object-builder-v1.mixins.json

@@ -13,6 +13,9 @@
     "MixinBlock",
     "PointOfInterestTypeAccessor",
     "SpawnRestrictionAccessor",
+    "TradeOffersAccessor",
+    "TradeOffersMixin",
+    "TypeAwareTradeMixin",
     "VillagerProfessionAccessor"
   ],
   "client": [

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

@@ -0,0 +1,37 @@
+/*
+ * 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.object.builder;
+
+import java.util.Random;
+
+import net.minecraft.entity.Entity;
+import net.minecraft.village.TradeOffer;
+import net.minecraft.village.TradeOffers;
+
+class SimpleTradeFactory implements TradeOffers.Factory {
+	private final TradeOffer offer;
+
+	SimpleTradeFactory(TradeOffer offer) {
+		this.offer = offer;
+	}
+
+	@Override
+	public TradeOffer create(Entity entity, Random random) {
+		// ALWAYS supply a copy of the offer.
+		return new TradeOffer(this.offer.toTag());
+	}
+}

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

@@ -0,0 +1,86 @@
+/*
+ * 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.object.builder;
+
+import static net.minecraft.command.argument.EntityArgumentType.entity;
+import static net.minecraft.command.argument.EntityArgumentType.getEntity;
+import static net.minecraft.server.command.CommandManager.argument;
+import static net.minecraft.server.command.CommandManager.literal;
+
+import java.util.Random;
+
+import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;
+
+import net.minecraft.entity.Entity;
+import net.minecraft.entity.passive.WanderingTraderEntity;
+import net.minecraft.item.ItemStack;
+import net.minecraft.item.Items;
+import net.minecraft.text.LiteralText;
+import net.minecraft.village.TradeOffer;
+import net.minecraft.village.TradeOffers;
+import net.minecraft.village.VillagerProfession;
+
+import net.fabricmc.api.ModInitializer;
+import net.fabricmc.fabric.api.command.v1.CommandRegistrationCallback;
+import net.fabricmc.fabric.api.object.builder.v1.trade.TradeOfferHelper;
+
+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.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)));
+		});
+
+		CommandRegistrationCallback.EVENT.register((dispatcher, dedicated) -> {
+			dispatcher.register(literal("fabric_refreshtrades").executes(context -> {
+				TradeOfferHelper.refreshOffers();
+				context.getSource().sendFeedback(new LiteralText("Refreshed trades"), false);
+				return 1;
+			}));
+
+			dispatcher.register(literal("fabric_applywandering_trades")
+					.then(argument("entity", entity()).executes(context -> {
+						final Entity entity = getEntity(context, "entity");
+
+						if (!(entity instanceof WanderingTraderEntity)) {
+							throw new SimpleCommandExceptionType(new LiteralText("Entity is not a wandering trader")).create();
+						}
+
+						WanderingTraderEntity trader = (WanderingTraderEntity) entity;
+						trader.getOffers().clear();
+
+						for (TradeOffers.Factory[] value : TradeOffers.WANDERING_TRADER_TRADES.values()) {
+							for (TradeOffers.Factory factory : value) {
+								final TradeOffer result = factory.create(trader, new Random());
+
+								if (result == null) {
+									continue;
+								}
+
+								trader.getOffers().add(result);
+							}
+						}
+
+						return 1;
+					})));
+		});
+	}
+}

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

@@ -0,0 +1,37 @@
+/*
+ * 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.object.builder;
+
+import net.minecraft.item.ItemStack;
+import net.minecraft.item.Items;
+import net.minecraft.village.TradeOffer;
+import net.minecraft.village.VillagerProfession;
+
+import net.fabricmc.api.ModInitializer;
+import net.fabricmc.fabric.api.object.builder.v1.trade.TradeOfferHelper;
+
+/*
+ * Second entrypoint to validate class loading does not break this.
+ */
+public class VillagerTypeTest2 implements ModInitializer {
+	@Override
+	public void onInitialize() {
+		TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
+			factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 20), new ItemStack(Items.NETHERITE_INGOT), 3, 4, 0.15F)));
+		});
+	}
+}

fabric-object-builder-api-v1/src/testmod/resources/fabric.mod.json

@@ -21,7 +21,9 @@
   "description": "Test mod for fabric object builder API v1.",
   "entrypoints": {
     "main": [
-      "net.fabricmc.fabric.test.object.builder.CriterionRegistryTest::init"
+      "net.fabricmc.fabric.test.object.builder.CriterionRegistryTest::init",
+      "net.fabricmc.fabric.test.object.builder.VillagerTypeTest1",
+      "net.fabricmc.fabric.test.object.builder.VillagerTypeTest2"
     ]
   }
 }