Crash with EasierVillagerTrade

[Pengu-Hunter]

Information

Minecraft version: 1.12.2
Forge version: 14.23.5.2854
Environment: Singleplayer

Mod name: Infinite Trade and EasierVillagerTrade
Mod version: 1.0 and 1.3

Description

Trying to trade with any villager vanilla or modded crashes the game I'm not sure which of the two mods is the source of the crash.

Crash report

crash-2020-04-30_15.32.25-client.txt

[gbl]

Hi, maker of the other mod (EasierVillagerTrading) here. As your source code isn't available, I can't really do much about this, but you're welcome to check my source code at https://github.com/gbl/EasierVillagerTrading/tree/legacy_1_12_2 - make sure to use the legacy_1_12_2 branch.

[ricksouth]

Hi Pengu and gbl.

I don't officially support 1.12 anymore, but I've looked into it. The code from 1.12 and 1.14+ work differently, but this is how I do it in 1.12:

@SubscribeEvent
public void onVillagerTrade(MerchantTradeOffersEvent e) {
	EntityPlayer player = e.getPlayer();
	if (player.getEntityWorld().isRemote) {
		return;
	}
	
	MerchantRecipeList recipes = e.getList();
	for (MerchantRecipe recipe : recipes) {
		recipe.increaseMaxTradeUses(99999);
	}
}

The crash report mentions line 15, which is:
if (player.getEntityWorld().isRemote) {

@gbl , do you know what causes the incompatibility issue? It seems like the player from MerchantTradeOffersEvent getPlayer is null. Is that something your code causes?

[Pengu-Hunter]

I saw that curse forge didn't have a 1.12 version listed only 1.13, 1.14 and 1.15, however when using twitch launcher Infinite Trading is available for download on 1.12 versions so I took that to mean it was working but I understand if it isn't supported anymore and if it is then I won't push you to make changes as I have found a different mod to EasierVillagerTrading that doesn't conflict but still accomplishes what I want, thank you both for replying.

Regards Pengu

[ricksouth]

The older versions are always available under the /files (https://www.curseforge.com/minecraft/mc-mods/infinite-trading/files). Newer versions get priority on the mod's main page.

Glad your issue is resolved, nonetheless @gbl let me know if I can do anything.

[Pengu-Hunter]

Good to know thanks for the help.

Regards Pengu

[gbl]

Ah, yes, that's me.
In order to display all of the recipes(=trades really) the villager has, on the same screen, I call getRecipes() on the merchant object. That method has a parameter player, which isn't ever used (all players got the same trades and prices in that version), so my code just passes null in there. Calling this method seems to make Forge emit a MerchantTradeOffersEvent.
Or course I could, and should, use Minecraft.getInstance().player instead.

By the way, I'm calling that method quite often (in the render loop, actually). This might cause a problem as you're incrementing the number of uses every time, and that value is an integer, so it'll overflow eventually.

But, fix this in an ancient 1.12 version? I'd say rather not.

[ricksouth]

The 1.14 version uses reflection to access the private trade fields and increases the amount that's tradable in there. As it doesn't seem a big issue I won't be updating the 1.12 version yea.

By the way, you should be able to call it 2147483648 / 99999 = 21475 times before the max integer value is reached :)

[ricksouth]

-- or just make sure to trade 99999 items every time it's called. Doesn't seem like a problem to me

[gbl]

And I switched to Fabric beginning with 1.14 so I guess there's never going to be an incompatibility between our mods again. But with some people flexing about their 200 fps, that's not even 2 minutes. Anyway, let's allow those old versions to rest in peace.

[ricksouth]

I believe Fabric has still planned on making it compatible with the forge loader in the future, but until then that's indeed the case.

Agreed, the older versions have served us well while they can. Becoming obsolete but forever in our hearts.

I'm considering this as solved, thanks everyone!