QuickBench mixin prevents vanilla ServerPlacer$triggerRecipeCrafted from firing when crafting

[TelepathicGrunt]

Environment

• Minecraft: 1.20.1
• QuickBench: v4.1.0+mc.1.20.1
• Loader: Fabric
• Java: 21
• Mods: Impacts Bumblezone

To Reproduce

I am the Dev of Bumblezone mod and I gotten this bug report:

TelepathicGrunt/Bumblezone#556

The bug is when users craft Beehives in a Crafting Table, my advancement that would've triggered after crafting a number of that block is not completing. This confused me because my mod mixins to triggerRecipeCrafted method in ServerPlayer to know when a recipe is crafted and count up for my advancement trigger. It should've been foolproof as it is tied basically to when vanilla's recipe crafter trigger is fired.

Bumblezone/common/src/main/java/com/telepathicgrunt/the_bumblezone/mixin/entities/ServerPlayerMixin.java

Upon pulling QuickBench into my dev environment I found that mod is redirecting RecipeHolder$awardUsedRecipes to call some custom code for recipe book recipe unlocking. But by doing so, it has circumvented calling ServerPlayer$triggerRecipeCrafted which means vanilla's CriteriaTriggers.RECIPE_CRAFTED advancement trigger never gets called. QuickBench effectively broke all recipe trigger advancement datapacks and mods.

The problematic mixin that needs to call RecipeHolder$awardUsedRecipes OR call ServerPlayer$triggerRecipeCrafted to restore functionality to anyone using recipe crafted trigger

QuickBench/src/main/java/tfar/fastbench/mixin/CraftingResultSlotMixin.java

Lines 76 to 86 in 661097b

	@Redirect(method = "checkTakeAchievements",
	at = @At(value = "INVOKE", target = "Lnet/minecraft/world/inventory/RecipeHolder;awardUsedRecipes(Lnet/minecraft/world/entity/player/Player;Ljava/util/List;)V"))
	public void no(final RecipeHolder instance, final Player player, final List<ItemStack> list) {
	if (((CraftingInventoryDuck) craftSlots).getCheckMatrixChanges() &&
	this.container instanceof RecipeHolder recipeHolder) {
	var recipeUsed = recipeHolder.getRecipeUsed();
	if (recipeUsed != null && !recipeUsed.isSpecial()) {
	player.awardRecipes(Collections.singleton(recipeUsed));
	}
	}
	}

In fact if you craft a pot using 4 sherds, vanilla should grant you and advancement but with QuickBench on, it does not:

With QuickBench removed, the pot crafting from sherds grants the advancement properly. You can use this to test if the updated code fixed the issue.

Expected results

Advancements using the recipe crafted trigger works properly

Logs, crash report, mod list or unmodified mod pack

N/A

[Ampflower]

Thanks for the report. Lemme see if I can optimally do it so that it only fires once as needed.

I may need to check whether stats works as intended as well, while at it.

[Ampflower]

Okay rereading the bug report: that might not be super optimal due to behaviour changes with regards to other mods. Ideally I could just tell the method how many times it has actually been fired for that, but that's not possible.

I think it'll require a config to allow toggling compatibility mode, with the default to sense known counters like yours.

[TelepathicGrunt]

Hmm since this mod is a derivative of FastWorkbench, maybe looking at how FastWorkbench works might be an idea as this bug does not happen with that mod as far as I know (I can test later)

I doubt players would want what appears to be a small optimization mod to straight up break the functionality of a ton of vanilla advancements. I also doubt the advancement trigger code is actually expensive to run so not running it isn’t going to save a lot of performance anyway.

[TelepathicGrunt]

Ok by test later, I mean now lol. Just tested FastWorkbench on 1.20.1 Forge and the sherd to pot crafting advancement triggered without issue.

I also tested this unofficial fabric port of FastWorkbench on 1.21.1 and the advancement triggers just fine for that too.

https://modrinth.com/mod/fastworkbench-fabric

Seems like an implementation issue specific to QuickBench that is not present on the original FastWorkbench mods