Truly-Modular / Modular-Item-API

A Modern Modular Item API
Other
26 stars 8 forks source link

Material repair ingredients have their matching stacks accessed before tag reload, bricking them. #29

Closed sisby-folk closed 9 months ago

sisby-folk commented 9 months ago

Ingredient caches the result of getMatchingStacks() when it's called, and it's possible to call the method before tags are actually loaded - this causes Ingredient to permanently contain the incorrect list of stacks from before the tag was actually loaded, breaking its usage in recipes.

miapi does this in MaterialProperty.java#L98-L108 with several calls to getMatchingStacks(), and truly modular armory calls test() GenerateArmorModularConverter.java#L43 in the same place via assignStats.

This is presumably because MiapiReloadListener is running before TagManagerLoader - when it needs to run after. If architectury allows you to set a dependency on the tag reloader, go ahead and do so.

The result is that the repair ingredient for wooden and stone tools becomes completely empty, preventing them from being repaired at an anvil.

We caught this because our own mod (Tinkerer's Smithing) has a mixin designed to prevent it from happening - here's a nice log of that happening for more detail and a stack trace: https://mclo.gs/NRksw99

sisby-folk commented 9 months ago

Here's a screenshot of a wooden pickaxe being non repairable when this mod is installed.

image

Smartin-b commented 9 months ago

huh? thats weird, reading a tag does brick it? well anyways guess ill just delay truly modulars reload logic until after the tags are setup

sisby-folk commented 9 months ago

You can look at the logic of Ingredient.getMatchingStacks() yourself - here's a screenshot image

it's a pretty silly hazard in vanilla.

Smartin-b commented 9 months ago

cool, ig i should just mixin into minecrafts reload logic and trigger after the initial loading is through to start miapis logic

sisby-folk commented 9 months ago

in fabric you have IdentifiableResourceReloadListener that handles this:

image

not sure what you have access to for forge.

Smartin-b commented 9 months ago

actually the architectury implementation supports dependencies, so ig ill use that

Smartin-b commented 9 months ago

So, hey, i tried architecturies implementation and while it seems to work ( i checked with mixins to verify the tag logic was executed prior to my logic) the issue perssisted. any ideas why that could be? otherwise im just gonna experiment arround until i find a fix

sisby-folk commented 9 months ago

try installing Tinkerer's Smithing 2.6 - it'll log if you're causing the issue

it'll also workaround the issue, so trust the log not the behaviour.

Smartin-b commented 9 months ago

i can check that im breaking it in local dev, if i comment my code out it stops breaking, i was just asking if you knew another reason that might be causing it other than beeing called prior to the tagloader

Smartin-b commented 9 months ago

nvmd, figured it out, we accessed stuff from the worker thread

Smartin-b commented 9 months ago

fixed in latest release