Skip to content

Commit 5048e74

Browse files
committed
Do not bypass Ingredient.TagValue#getItems during server resource reload
This breaks KubeJS/CraftTweaker's workarounds for tags not being available during RecipeManager#apply (even though they become available right after) Related: #555
1 parent c3086e5 commit 5048e74

File tree

4 files changed

+90
-4
lines changed

4 files changed

+90
-4
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package org.embeddedt.modernfix.forge.load;
2+
3+
public class MinecraftServerReloadTracker {
4+
public static int ACTIVE_RELOADS = 0;
5+
6+
public static boolean isReloadActive() {
7+
return ACTIVE_RELOADS > 0;
8+
}
9+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package org.embeddedt.modernfix.forge.mixin.core;
2+
3+
import com.llamalad7.mixinextras.injector.ModifyExpressionValue;
4+
import net.minecraft.server.MinecraftServer;
5+
import org.embeddedt.modernfix.forge.load.MinecraftServerReloadTracker;
6+
import org.spongepowered.asm.mixin.Mixin;
7+
import org.spongepowered.asm.mixin.injection.At;
8+
import org.spongepowered.asm.mixin.injection.Inject;
9+
import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
10+
11+
import java.util.Collection;
12+
import java.util.concurrent.CompletableFuture;
13+
import java.util.concurrent.Executor;
14+
15+
@Mixin(MinecraftServer.class)
16+
public class MinecraftServerMixin {
17+
@Inject(method = "reloadResources", at = @At("HEAD"))
18+
private void startReloadTrack(Collection<String> selectedIds, CallbackInfoReturnable<CompletableFuture<Void>> cir) {
19+
MinecraftServerReloadTracker.ACTIVE_RELOADS++;
20+
}
21+
22+
@ModifyExpressionValue(method = "reloadResources", at = @At(value = "INVOKE", target = "Ljava/util/concurrent/CompletableFuture;thenAcceptAsync(Ljava/util/function/Consumer;Ljava/util/concurrent/Executor;)Ljava/util/concurrent/CompletableFuture;", ordinal = 0))
23+
private CompletableFuture<Void> mfix$endReloadTrack(CompletableFuture<Void> original) {
24+
return original.thenAcceptAsync(val -> {
25+
MinecraftServerReloadTracker.ACTIVE_RELOADS--;
26+
}, (Executor)this);
27+
}
28+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.embeddedt.modernfix.forge.mixin.core;
2+
3+
import com.llamalad7.mixinextras.injector.ModifyReturnValue;
4+
import com.llamalad7.mixinextras.sugar.Local;
5+
import net.minecraft.server.WorldLoader;
6+
import org.embeddedt.modernfix.forge.load.MinecraftServerReloadTracker;
7+
import org.spongepowered.asm.mixin.Mixin;
8+
import org.spongepowered.asm.mixin.injection.At;
9+
import org.spongepowered.asm.mixin.injection.Inject;
10+
import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
11+
12+
import java.util.concurrent.CompletableFuture;
13+
import java.util.concurrent.Executor;
14+
15+
@Mixin(WorldLoader.class)
16+
public class WorldLoaderMixin {
17+
@Inject(method = "load", at = @At("HEAD"))
18+
private static void trackStartReload(CallbackInfoReturnable<CompletableFuture<?>> cir) {
19+
MinecraftServerReloadTracker.ACTIVE_RELOADS++;
20+
}
21+
22+
@ModifyReturnValue(method = "load", at = @At("RETURN"))
23+
private static <V> CompletableFuture<V> trackEndReload(CompletableFuture<V> original, @Local(ordinal = 1, argsOnly = true) Executor syncExecutor) {
24+
return original.thenApplyAsync(val -> {
25+
MinecraftServerReloadTracker.ACTIVE_RELOADS--;
26+
return val;
27+
}, syncExecutor);
28+
}
29+
}

forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/faster_ingredients/IngredientMixin.java

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import net.minecraft.core.registries.BuiltInRegistries;
77
import net.minecraft.world.item.ItemStack;
88
import net.minecraft.world.item.crafting.Ingredient;
9+
import org.embeddedt.modernfix.forge.load.MinecraftServerReloadTracker;
910
import org.embeddedt.modernfix.forge.recipe.ExtendedIngredient;
1011
import org.embeddedt.modernfix.forge.recipe.IngredientItemStacksSoftReference;
1112
import org.jetbrains.annotations.Nullable;
@@ -34,13 +35,32 @@ public abstract class IngredientMixin implements ExtendedIngredient {
3435

3536
private volatile IngredientItemStacksSoftReference mfix$cachedItemStacks;
3637

38+
/**
39+
* Minecraft's server resource loading process has a design flaw in that tags are loaded, recipes are loaded,
40+
* then tags are bound to the server registries. This results in recipe modification mods like KubeJS/CraftTweaker
41+
* not being able to use Ingredient.test reliably as the TagValue will not find any contents for the given tag.
42+
* To work around this issue these mods track tag context themselves and then patch TagValue.getItems to use it
43+
* during the resource reload process. We often bypass Value.getItems, so we must disable that bypass
44+
* whenever a server reload is in progress.
45+
* <p>
46+
* An alternative fix would be to bind tags ourselves when the recipe manager reload begins, before control is
47+
* handed to these mods. However, it's unclear if there would be any negative side effects from binding tags early
48+
* like this. Moreover, this fix would only work if the mod-provided patches to getItems read exactly what the
49+
* registry would normally contain, rather than a modified version.
50+
* <p>
51+
* Note: this is a separate problem from the issue where clients may receive recipes before tags in 1.21.
52+
*/
53+
private boolean mfix$areTagsAvailable() {
54+
return !MinecraftServerReloadTracker.isReloadActive();
55+
}
56+
3757
/**
3858
* @author embeddedt
3959
* @reason tag ingredients can be tested without iterating over all items
4060
*/
4161
@Inject(method = "test(Lnet/minecraft/world/item/ItemStack;)Z", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/item/crafting/Ingredient;getItems()[Lnet/minecraft/world/item/ItemStack;"), cancellable = true)
4262
private void modernfix$fasterTagIngredientTest(ItemStack stack, CallbackInfoReturnable<Boolean> cir) {
43-
if (this.isVanilla() && this.values.length == 1 && this.values[0] instanceof Ingredient.TagValue tagValue) {
63+
if (this.isVanilla() && this.values.length == 1 && this.values[0] instanceof Ingredient.TagValue tagValue && mfix$areTagsAvailable()) {
4464
cir.setReturnValue(stack.getItemHolder().is(tagValue.tag));
4565
}
4666
}
@@ -60,7 +80,7 @@ private boolean containsItems() {
6080
for (Ingredient.Value value : this.values) {
6181
if (value instanceof Ingredient.ItemValue) {
6282
return true;
63-
} else if (value instanceof Ingredient.TagValue tagValue) {
83+
} else if (value instanceof Ingredient.TagValue tagValue && mfix$areTagsAvailable()) {
6484
var holderSetOpt = BuiltInRegistries.ITEM.getTag(tagValue.tag);
6585
if (holderSetOpt.isPresent() && holderSetOpt.get().size() > 0) {
6686
return true;
@@ -84,7 +104,7 @@ private boolean containsItems() {
84104
*/
85105
@Inject(method = "getStackingIds", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/item/crafting/Ingredient;getItems()[Lnet/minecraft/world/item/ItemStack;"), cancellable = true)
86106
private void modernfix$fasterTagIngredientStacking(CallbackInfoReturnable<IntList> cir) {
87-
if (this.isVanilla() && this.values.length == 1 && this.values[0] instanceof Ingredient.TagValue tagValue) {
107+
if (this.isVanilla() && this.values.length == 1 && this.values[0] instanceof Ingredient.TagValue tagValue && mfix$areTagsAvailable()) {
88108
var tag = BuiltInRegistries.ITEM.getTag(tagValue.tag);
89109
if (!tag.isPresent() || tag.get().size() == 0) {
90110
return;
@@ -124,7 +144,7 @@ private ItemStack[] computeItemsArray() {
124144
if (this.values.length == 1) {
125145
if (this.values[0] instanceof Ingredient.ItemValue itemValue) {
126146
return new ItemStack[] { itemValue.item };
127-
} else if (this.values[0] instanceof Ingredient.TagValue tagValue) {
147+
} else if (this.values[0] instanceof Ingredient.TagValue tagValue && mfix$areTagsAvailable()) {
128148
var tag = BuiltInRegistries.ITEM.getTag(tagValue.tag);
129149
if (tag.isPresent() && tag.get().size() > 0) {
130150
var holderSet = tag.get();

0 commit comments

Comments
 (0)