Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Better loottable hooks #139

Draft
wants to merge 2 commits into
base: 1.14.x
Choose a base branch
from
Draft

Better loottable hooks #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01c8f20
99% of the way to the alternative implementation I've been talking about
kitlith Jul 22, 2020
a9d9dff
It's totally feasible now. :)
kitlith Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 46 additions & 91 deletions patchwork-loot/src/main/java/net/patchworkmc/impl/loot/LootHooks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,12 @@

package net.patchworkmc.impl.loot;

import java.util.Deque;
import java.util.HashSet;

import javax.annotation.Nullable;

import com.google.common.collect.Queues;
import com.google.common.collect.Sets;
import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParseException;
import org.spongepowered.asm.mixin.Unique;
import org.apache.commons.lang3.mutable.MutableInt;

import net.minecraft.loot.LootManager;
import net.minecraft.loot.LootTable;
Expand All @@ -39,111 +33,72 @@

import net.patchworkmc.impl.event.loot.LootEvents;

// NOTE: this class is more or less a direct copy of parts of Forge's ForgeHooks.
public class LootHooks {
@Unique
private static ThreadLocal<Deque<LootTableContext>> lootContext = new ThreadLocal<Deque<LootTableContext>>();

public static LootTable loadLootTable(Gson gson, Identifier name, JsonElement data, boolean custom, LootManager lootTableManager) {
Deque<LootTableContext> que = lootContext.get();

if (que == null) {
que = Queues.newArrayDeque();
lootContext.set(que);
}

LootTable ret = null;

try {
que.push(new LootTableContext(name, custom));
ret = gson.fromJson(data, LootTable.class);
que.pop();
} catch (JsonParseException e) {
que.pop();
throw e;
}

if (!custom) {
ret = LootEvents.loadLootTable(name, ret, lootTableManager);
}

// if (ret != null) {
// ret.freeze();
// }

return ret;
}

private static LootTableContext getLootTableContext() {
LootTableContext ctx = lootContext.get().peek();

if (ctx == null) {
throw new JsonParseException("Invalid call stack, could not grab json context!"); // Should I throw this? Do we care about custom deserializers outside the manager?
public static void prepareLootTable(Identifier id, JsonObject lootTableObj, boolean custom, LootManager lootTableManager) {
// passing pool names into loot pool deserialization mixin since we have all the information we need right now, but not later
if (lootTableObj.has("pools")) {
MutableInt poolCount = new MutableInt(0);
JsonArray pools = lootTableObj.getAsJsonArray("pools");

for (JsonElement elem: pools) {
JsonObject pool = JsonHelper.asObject(elem, "loot pool");
String name = getPoolName(pool, id, custom, poolCount);
pool.addProperty("name", name);
}
}

return ctx;
// we don't call GSON here because we're letting vanilla do it in a bit.
// ForgeHooks implementation wraps this, performing the deserialization
}

public static String readPoolName(JsonObject json) {
LootTableContext ctx = LootHooks.getLootTableContext();
ctx.resetPoolCtx();
// More or less a copy of ForgeHooks.readPoolName. Actual implementation of that will just pull the name
// from the json, since prepareLootTable is taking the name from this and putting it in the json.
private static String getPoolName(JsonObject lootPoolObj, Identifier id, boolean custom, MutableInt poolCount) {
boolean vanilla = "minecraft".equals(id.getNamespace());

if (json.has("name")) {
return JsonHelper.getString(json, "name");
if (lootPoolObj.has("name")) {
return JsonHelper.getString(lootPoolObj, "name");
}

if (ctx.custom) {
return "custom#" + json.hashCode(); //We don't care about custom ones modders shouldn't be editing them!
if (custom) {
return "custom#" + lootPoolObj.hashCode(); //We don't care about custom ones modders shouldn't be editing them!
}

ctx.poolCount++;
poolCount.increment();

if (!ctx.vanilla) {
if (!vanilla) {
throw new JsonParseException("Loot Table \"" + ctx.name.toString() + "\" Missing `name` entry for pool #" + (ctx.poolCount - 1));
}

return ctx.poolCount == 1 ? "main" : "pool" + (ctx.poolCount - 1);
return poolCount.intValue() == 1 ? "main" : "pool" + (poolCount.intValue() - 1);
}

private static class LootTableContext {
public final Identifier name;
public final boolean custom;
private final boolean vanilla;
public int poolCount = 0;
public int entryCount = 0;
private HashSet<String> entryNames = Sets.newHashSet();

private LootTableContext(Identifier name, boolean custom) {
this.name = name;
this.custom = custom;
this.vanilla = "minecraft".equals(this.name.getNamespace());
}
// TODO: should we move this implementation to ForgeHooks? since this is only going to be called by ForgeHooks.
public static LootTable loadLootTable(Gson gson, Identifier name, JsonElement data, boolean custom, LootManager lootTableManager) {
JsonObject lootTableObj = JsonHelper.asObject(data, "loot table");

private void resetPoolCtx() {
this.entryCount = 0;
this.entryNames.clear();
}
// This accomplishes the stashing of data that Forge does, without needing a threadlocal static
prepareLootTable(name, lootTableObj, custom, lootTableManager);

public String validateEntryName(@Nullable String name) {
if (name != null && !this.entryNames.contains(name)) {
this.entryNames.add(name);
return name;
}
// TODO: Can we do something like call the lambda method directly?
// As is, we're gonna duplicate the stuff we add via mixin.

if (!this.vanilla) {
throw new JsonParseException("Loot Table \"" + this.name.toString() + "\" Duplicate entry name \"" + name + "\" for pool #" + (this.poolCount - 1) + " entry #" + (this.entryCount - 1));
}
LootTable ret = gson.fromJson(data, LootTable.class);

int x = 0;
if (!custom) {
ret = LootEvents.loadLootTable(name, ret, lootTableManager);
}

while (this.entryNames.contains(name + "#" + x)) {
x++;
}
// if (ret != null) {
// ret.freeze();
// }

name = name + "#" + x;
this.entryNames.add(name);
return ret;
}

return name;
}
// Replacement for ForgeHooks.readPoolName: since we prepared the json object, the name is already present.
public static String readPoolName(JsonObject lootPoolObj) {
// TODO: throw a better exception?
return JsonHelper.getString(lootPoolObj, "name");
}
}
30 changes: 30 additions & 0 deletions patchwork-loot/src/main/java/net/patchworkmc/impl/loot/WrappedImmutableMapBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package net.patchworkmc.impl.loot;

import java.util.function.BiFunction;

import javax.annotation.ParametersAreNonnullByDefault;

import com.google.common.collect.ImmutableMap;

import net.patchworkmc.mixin.loot.MixinLootManager;

/**
* A builder for creating immutable map instances that applies some sort of transformation
* before putting items into the map. See {@link ImmutableMap.Builder} for normal usage.
*
* <p>Yes, this is a hack, see usage in {@link MixinLootManager}</p>
*/
@ParametersAreNonnullByDefault
public class WrappedImmutableMapBuilder<K, V> extends ImmutableMap.Builder<K, V> {
private final BiFunction<K, V, V> wrap;

public WrappedImmutableMapBuilder(BiFunction<K, V, V> wrap) {
super();
this.wrap = wrap;
}

@Override
public ImmutableMap.Builder<K, V> put(K key, V value) {
return super.put(key, wrap.apply(key, value));
}
}
49 changes: 31 additions & 18 deletions patchwork-loot/src/main/java/net/patchworkmc/mixin/loot/MixinLootManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,21 @@

package net.patchworkmc.mixin.loot;

import java.io.IOException;
import java.util.Map;
import java.util.function.BiConsumer;

import com.google.common.collect.ImmutableMap;
import com.google.gson.Gson;
import com.google.gson.JsonObject;
import org.apache.logging.log4j.Logger;
import org.spongepowered.asm.mixin.Final;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Overwrite;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.Unique;
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.CallbackInfo;
import org.spongepowered.asm.mixin.injection.callback.LocalCapture;

import net.minecraft.loot.LootManager;
import net.minecraft.loot.LootTable;
Expand All @@ -43,7 +42,9 @@
import net.minecraft.util.Identifier;
import net.minecraft.util.profiler.Profiler;

import net.patchworkmc.impl.event.loot.LootEvents;
import net.patchworkmc.impl.loot.LootHooks;
import net.patchworkmc.impl.loot.WrappedImmutableMapBuilder;

@Mixin(LootManager.class)
public abstract class MixinLootManager extends MixinJsonDataLoader {
Expand All @@ -55,27 +56,39 @@ public abstract class MixinLootManager extends MixinJsonDataLoader {
@Final
private static Logger LOGGER;

@Redirect(method = "apply", at = @At(value = "INVOKE", target = "java/util/Map.forEach (Ljava/util/function/BiConsumer;)V", ordinal = 0))
private void cancel_forEach(Map<Identifier, JsonObject> map, BiConsumer<Identifier, JsonObject> consumer) {
// ignore this call, we're gonna reintroduce it but with capturing locals
}

@Inject(method = "apply", at = @At(value = "INVOKE", target = "java/util/Map.forEach (Ljava/util/function/BiConsumer;)V", ordinal = 0), locals = LocalCapture.CAPTURE_FAILHARD)
private void reintroduce_forEach(Map<Identifier, JsonObject> map, ResourceManager resourceManager, Profiler profiler, CallbackInfo info, ImmutableMap.Builder<Identifier, LootTable> builder) {
map.forEach((id, jsonObject) -> {
// NOTE: this could also be a Redirect of forEach that just wraps the existing lambda, instead of an additional forEach
@Inject(method = "apply", at = @At(value = "INVOKE", target = "java/util/Map.forEach (Ljava/util/function/BiConsumer;)V", ordinal = 0))
private void prepareJson(Map<Identifier, JsonObject> map, ResourceManager resourceManager, Profiler profiler, CallbackInfo info) {
map.forEach((id, lootTableObj) -> {
try {
boolean custom = isLootTableCustom(resourceManager, id);
LootManager lootManager = (LootManager) (Object) this;
Resource res = resourceManager.getResource(this.getPreparedPath(id));
LootTable lootTable = LootHooks.loadLootTable(GSON, id, jsonObject, res == null || !res.getResourcePackName().equals("Default"), lootManager);
builder.put(id, lootTable);
} catch (Exception ex) {
LootHooks.prepareLootTable(id, lootTableObj, custom, lootManager);
} catch (IOException ex) {
LOGGER.error("Couldn't parse loot table {}", id, ex);
}
});
}

@Overwrite
private static void method_20711(ImmutableMap.Builder<Identifier, LootTable> builder, Identifier id, JsonObject obj) {
// We are effectively overwriting this lambda with our own, so let's make that explicit by actually overwriting it.
@Redirect(method = "apply", at = @At(value = "INVOKE", target = "com/google/common/collect/ImmutableMap.builder ()Lcom/google/common/collect/ImmutableMap$Builder;"))
private ImmutableMap.Builder<Identifier, LootTable> wrapBuilder(Map<Identifier, JsonObject> map, ResourceManager resourceManager, Profiler profiler) {
LootManager lootManager = (LootManager) (Object) this;
return new WrappedImmutableMapBuilder<Identifier, LootTable>((id, table) -> {
// TODO: handle exception?
if (!isLootTableCustom(resourceManager, id)) {
table = LootEvents.loadLootTable(id, table, lootManager);
}

// if (table != null) {
// table.freeze();
// }
return table;
});
}

@Unique
private boolean isLootTableCustom(ResourceManager resourceManager, Identifier id) throws IOException {
Resource res = resourceManager.getResource(this.getPreparedPath(id));
return res == null || !res.getResourcePackName().equals("Default");
}
}