Malorolam / LootBags

Other
21 stars 32 forks source link

Does your mod make exceptions for certain mod's NBT information #43

Closed DoomSquirter closed 7 years ago

DoomSquirter commented 7 years ago

Trying to help figure out a many-mod issue, the concise version is as follows:

Wondering if you have already put some exceptions in your code for other mods? If so, this is one mod that needs that exception handled I guess (everlasting abilities). If you don't make exceptions, then possibly the problem is in the other mod(s).

Thanks for your time

Edit: this is associated to #39 which you closed already. Should have checked closed tickets first, but wondering if what I said above changes your thoughts on that?

Malorolam commented 7 years ago

Everlasting Abilties I've looked at before, as far as I've found, the error is on their end. They're using the MC loot functionality added in 1.9, which is being called and applied correctly by my code, as seen with the enchanting books, quark enchanting books, and enchanting plus scrolls, which do the same thing and they all work fine.

I am not inclined to add in code for only one mod, especially when I have multiple examples of the same functionality working elsewhere with what I have already.

DoomSquirter commented 7 years ago

That's exactly what I was trying to nail down. It did seem that lootbags does the enchanting plus scrolls properly, thus something's amiss with the everlasting abilities totems I think. Thanks alot for the input!

rubensworks commented 7 years ago

@Malorolam I just had a look at your source code, and it looks like you are not correctly using the loot functions.

As can be seen here: https://github.com/Malorolam/LootBags/blob/5eeade2729d1d60d1c354c47a5a1b9e58cb0c8b9/net/minecraft/world/storage/loot/LootEntryItemAccess.java#L61 each apply invocation returns a certain ItemStack, but the problem is that this returned ItemStack is not necessarily the same as the input ItemStack. In fact, your method there completely ignores the returned ItemStacks.

Ideally, this piece of code should override the source ItemStack.

I will make some adjustments to Everlasting Abilities to make sure this won't be a problem anymore, but eitherway, this should be fixed on your end as well, because you might run into other issues as well ;-)

Malorolam commented 7 years ago

From what I've seen in the vanilla code for the specific loot functions, and thus the example most people would likely use as reference, the returned apply function is the modified ItemStack. Which is exactly what that code does.
As for the second code fragment, that code continues to work as long as the implementation of the loot function modifies the passed stack instead of copying it, then modifying the copy. It'll get fixed in the next version.