Productive Bees

Productive Bees

10M Downloads

Bug [1.16.5] - Causes removal of village loot tables

ozokuz opened this issue · 20 comments

commented

I haven't tested if this affects vanilla villages, but at least it affects ones from repurposed structures

Log

commented

@ozokuz Im the dev of repurposed structures. My mod does not use vanilla loot tables. Instead, what I do is all my villages has their own loot tables and when you open my chests, I roll my loot table, get that loot, then roll the corresponding vanilla loot table, grab the modded items from that loot table, and combine it with the output of my loot table.

So as a result, opening a Dark Forest Village’s house chest will have loot from its own dark forest village house loot table and also have modded items from vanilla’s plain village house loot table.

So this leads to questions about what exactly are you seeing. What do mean by “removal” of loot tables? Which chests are you opening in what structure? Is the issue reproducible with just repurposed structures and productive bees?

commented

Looking at the log, I think this is about a crash. Not so much removal of loot tables but that the loot table code for productive bees isn’t null safe. Specially, event.getTable().getPool("main") and maybe the getName(). Just sanity checking for null there should probably prevent the issue from occurring since other mods and datapacks can change the vanilla loot table and make the “main” pool no longer exist which then causes a null crash

public static void onLootSetup(LootTableLoadEvent event) {

commented

Another mod is breaking the loot table I am adding stuff to, can you share your entire log please to see if it's possible to identify it?

commented

Have you reproduced this with just repurposed structures and productive bees?

What other mods are you having in your setup?

commented

What do mean by “removal” of loot tables?

I mean that the loot tables don't appear in game at all. Doing /loot spawn ~ ~ ~ loot repurposed_structures:chests/village/ results in it not autocompleting at all.

Looking at the log, I think this is about a crash.

It's not a crash, just forge catching the error and the loot table not being added at all.

Show the latest.log. Also, this exception here isn’t actually going to break my loot table because that loot table event being used does not fire for modded loot tables. In fact, like I mentioned before, my loot table is separate from vanilla loot tables. It’s when you go to open a chest with my loot table that I run code to try and spawn loot from vanilla’s loot table and that’s when this exception is throw because someone did modify the vanilla loot table.

This is why I’m asking for the latest.log as if you are correct that my loot tables no longer exist, then that’s an entirely different issue that isn’t related to productive bees. I’m hoping the latest.log might have info on loot tables dying at world load. Hopefully. (Also, does that command actually autocomplete for modded loot tables in the first place?)

commented

Your mod is breaking the loot tables, not another mod.

What TelepathicGrunt said is what is happening here.
event.getTable().getPool("main") may return null.

I tried to backport the loot modifier stuff you wrote for 1.18 as the way you're adding loot in 1.16 is a bit hacky, but I ran into other issues when building.

commented

I get that it’s my mod throwing an error but it’s because someone else is messing up the main loot pool. If you’re trying to fix it yourself you can just do a null check.

commented

What do mean by “removal” of loot tables?

I mean that the loot tables don't appear in game at all. Doing /loot spawn ~ ~ ~ loot repurposed_structures:chests/village/ results in it not autocompleting at all.

Looking at the log, I think this is about a crash.

It's not a crash, just forge catching the error and the loot table not being added at all.

commented

If you’re trying to fix it yourself you can just do a null check.

I tried to do that, but it won't build.

It gives an error entries has private access in net.minecraft.loot.LootPool
I asked about it in the forge discord and they said it's because the field is part of a forge patch so your access transformer will not work, and they told to use a loot modifier instead.

commented

Here is the command autocomplete with no productive bees:
https://cdn.discordapp.com/attachments/583023149801799703/974640875814670406/javaw_6FdV5Z6YI0.png

And here is the command autocomplete with productive bees:
https://cdn.discordapp.com/attachments/583023149801799703/974643043149951066/javaw_mUcfARURgA.png

And here is the full latest.log:
https://gist.github.com/ozokuz/51d3166e1ea000909212cf4694eb8300
If you search for repurposed_structures:chests/village you'll find the errors

commented

Wtf. So from what I can tell, that loot table event is supposed to only fire for vanilla loot tables as shown here.
image

But here in your stacktrace, LootTablesKJS shows up where KubeJS is... maunally firing that forge event for all loot tables including modded ones?
image

And that crashes because now Productive Bee's initial check for village loot tables paths will match Repurposed Structure's village loot table paths and my loot table files doesn't have a named "main" pool like vanilla's does. Thus the crash. I bet removing KubeJS would prevent the crash. Productive Bees should also add a check for the minecraft namespace as well along with checking the path.

commented

productivebees-1.16.5-0.6.9.26.zip

Try this, unzipped. Let me know if it works.

commented

So this is not an issue solely related to productive bees, but to kubejs.
Shall I report this in the kubejs repo and reference this issue?

commented

they probably have their reasons for what they are doing

this code is old and jank anyways and not in use any longer for the 1.18 branch, I'll put up a new version with a null check as soon as I get my local env working for the 1.16 branch

commented

@ozokuz I talked with kubejs dev and they are going to investigate why the event is firing from their code. I also heard from other devs that enigmatic legacy mod also fires the event for all loot tables so keep that in mind

commented

I'll put up a new version with a null check as soon as I get my local env working for the 1.16 branch

Awesome, will re add productive bees to the pack when the new version is up.

I talked with kubejs dev and they are going to investigate why the event is firing from their code.

Ok

I also heard from other devs that enigmatic legacy mod also fires the event for all loot tables so keep that in mind

I've used that mod before but it's not in this pack. Thanks for the info though!

commented

@ozokuz did you have a chance to try it out?

commented

Sorry, been busy. Will check it tomorrow.

commented

Hey, finally got around to trying it out.
It works great.

commented

awesome thank, I'll post it then