Fix trade offers by writing to the trade map directly (#1430)

* Write to the trade map directly

* Add warning when TradeOfferHelper#refreshOffers is called

* Correctly use synchronized, just in case

* Add null check - PR should be ready for merge now

* Make ctor private to hide it from javadoc
This commit is contained in:
Technici4n 2021-05-01 23:04:53 +02:00 committed by Player
parent 4e7237c745
commit 1f1ad061d4
7 changed files with 49 additions and 172 deletions

View file

@ -7,6 +7,8 @@ dependencies {
moduleDependencies(project, [ moduleDependencies(project, [
'fabric-api-base', 'fabric-api-base',
'fabric-resource-loader-v0',
'fabric-tag-extensions-v0',
'fabric-tool-attribute-api-v1' 'fabric-tool-attribute-api-v1'
]) ])

View file

@ -57,11 +57,13 @@ public final class TradeOfferHelper {
} }
/** /**
* Refreshes the trade list by resetting the trade lists to vanilla state, and then registering all trade offers again. * @deprecated This never did anything useful.
*
* <p>This method is geared for use by mods which for example provide data driven villager trades.
*/ */
@Deprecated
public static void refreshOffers() { public static void refreshOffers() {
TradeOfferInternals.refreshOffers(); TradeOfferInternals.printRefreshOffersWarning();
}
private TradeOfferHelper() {
} }
} }

View file

@ -17,106 +17,50 @@
package net.fabricmc.fabric.impl.object.builder; package net.fabricmc.fabric.impl.object.builder;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Objects;
import java.util.function.Consumer; import java.util.function.Consumer;
import it.unimi.dsi.fastutil.ints.Int2ObjectMap; import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.ArrayUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import net.minecraft.village.TradeOffers; import net.minecraft.village.TradeOffers;
import net.minecraft.village.VillagerProfession; import net.minecraft.village.VillagerProfession;
import net.fabricmc.fabric.mixin.object.builder.TradeOffersAccessor;
public final class TradeOfferInternals { public final class TradeOfferInternals {
/** private static final Logger LOGGER = LogManager.getLogger("fabric-object-builder-api-v1");
* 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() { private TradeOfferInternals() {
} }
public static void registerVillagerOffers(VillagerProfession profession, int level, Consumer<List<TradeOffers.Factory>> factory) { // 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) {
Objects.requireNonNull(profession, "VillagerProfession may not be null.");
registerOffers(TradeOffers.PROFESSION_TO_LEVELED_TRADE.computeIfAbsent(profession, key -> new Int2ObjectOpenHashMap<>()), level, factory);
}
public static synchronized void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) {
registerOffers(TradeOffers.WANDERING_TRADER_TRADES, level, factory);
}
// 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<>(); final List<TradeOffers.Factory> list = new ArrayList<>();
factory.accept(list); factory.accept(list);
final TradeOffers.Factory[] additionalEntries = list.toArray(new TradeOffers.Factory[0]); final TradeOffers.Factory[] originalEntries = leveledTradeMap.computeIfAbsent(level, key -> new TradeOffers.Factory[0]);
final Int2ObjectMap<TradeOffers.Factory[]> professionEntry = VILLAGER_TRADE_FACTORIES.computeIfAbsent(profession, p -> new Int2ObjectOpenHashMap<>()); final TradeOffers.Factory[] addedEntries = list.toArray(new TradeOffers.Factory[0]);
final TradeOffers.Factory[] currentEntries = professionEntry.computeIfAbsent(level, l -> new TradeOffers.Factory[0]); final TradeOffers.Factory[] allEntries = ArrayUtils.addAll(originalEntries, addedEntries);
final TradeOffers.Factory[] newEntries = ArrayUtils.addAll(additionalEntries, currentEntries); leveledTradeMap.put(level, allEntries);
professionEntry.put(level, newEntries);
// Refresh the trades map
TradeOfferInternals.refreshOffers();
} }
public static void registerWanderingTraderOffers(int level, Consumer<List<TradeOffers.Factory>> factory) { public static void printRefreshOffersWarning() {
final List<TradeOffers.Factory> list = new ArrayList<>(); Throwable loggingThrowable = new Throwable();
factory.accept(list); LOGGER.warn("TradeOfferHelper#refreshOffers does not do anything, yet it was called! Stack trace:", loggingThrowable);
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();
} }
} }

View file

@ -1,39 +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.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!");
}
}

View file

@ -1,45 +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.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;
}
}

View file

@ -13,8 +13,6 @@
"MixinBlock", "MixinBlock",
"PointOfInterestTypeAccessor", "PointOfInterestTypeAccessor",
"SpawnRestrictionAccessor", "SpawnRestrictionAccessor",
"TradeOffersAccessor",
"TradeOffersMixin",
"TypeAwareTradeMixin", "TypeAwareTradeMixin",
"VillagerProfessionAccessor", "VillagerProfessionAccessor",
"VillagerTypeAccessor" "VillagerTypeAccessor"

View file

@ -31,7 +31,22 @@ public class VillagerTypeTest2 implements ModInitializer {
@Override @Override
public void onInitialize() { public void onInitialize() {
TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> { 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))); factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 5), new ItemStack(Items.NETHERITE_INGOT), 3, 4, 0.15F)));
});
TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 6), new ItemStack(Items.ELYTRA), 3, 4, 0.15F)));
});
TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 7), new ItemStack(Items.CHAINMAIL_BOOTS), 3, 4, 0.15F)));
});
TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 8), new ItemStack(Items.CHAINMAIL_CHESTPLATE), 3, 4, 0.15F)));
});
TradeOfferHelper.registerVillagerOffers(VillagerProfession.ARMORER, 1, factories -> {
factories.add(new SimpleTradeFactory(new TradeOffer(new ItemStack(Items.DIAMOND, 9), new ItemStack(Items.CHAINMAIL_HELMET), 3, 4, 0.15F)));
});
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)));
}); });
} }
} }