crazysnailboy / VillagerTrades

A minecraft mod to manipulate villager trades using JSON files
MIT License
5 stars 6 forks source link

Ore dictionary support #5

Open Flabort opened 7 years ago

Flabort commented 7 years ago

As was realized in a now closed issue, this mod does not support ore dictionary.

It would be nice if, for the sell option, we could put in an oredictionary component for the item id, or instead of an item id, and a villager with that trade would be able to accept anything within that ore dictionary. I'd like to be able to sell any ore:woodPlanks (so the vanilla planks, BoP planks, etc) to a villager within one trade definition.

Right now we'd have to define every item in the dictionary, either as separate trades or as options within a trade.

crazysnailboy commented 7 years ago

I've had a look into this - it turns out it's quite complicated!

I can read ore dictionary values from the JSON files and create a valid MerchantRecipe class which is ore dictionary aware, but then there's a lot of vanilla code involved in villager trades which doesn't use the ore dictionary that needs to be changed or overridden. I added a custom container, inventory and GUI which all work, the main problems now are the MerchantRecipeList which isn't designed to be extensible, and errors which are thrown when saving villager NBT data.

I'm going to leave this open and see if I can find a way around it. It may require some ASM (i.e. core modding), in which case I wouldn't do it as part of this mod but I've been considering a sister mod to this one which could include various core modding changes I'd like to make to villagers, and then if both VTT and "VTT Core" are installed, VTT could contain additional functionality which relies on "VTT Core" (e.g. Ore Dictionary support). This is a last resort though - I'd rather avoid using ASM if I can.

Flabort commented 7 years ago

Would it be possible to hook into zenscript/crafttweaker for that workaround? So when you want to use oredict in a villager trade, you need to have a script, and the JSON reads the script?

Ooh, if that were possible, you could define just... some sort of trade_object class for zenscript, so you define such an object in a .zs file (lets say it looks like "mods.vtt.addTradeObject("myObject", [,]);", then in the JSON file for a villager you could use "vtt:myObject" as an item to be traded, and then if something updates the zenscript profile (like ftb tweaks switching pack modes) then a villager with the trade_object automatically switches objects.

Yeah, you're right, that's probably not going to happen, especially without vtt core.

crazysnailboy commented 7 years ago

Unfortunately I don't think using ZenScript would help, as it would be subject to the same problems of having to save and load ore dictionary aware recipes from the villager that I'm having now.

That said, I've recently added ZenScript support to a different mod of mine and it had occured to me that a CraftTweaker addon for villager trades could be a thing - "VillagerTweaker", if you will. I might investigate that...

Flabort commented 7 years ago

To be honest, I thought VTT was already doing core modding, since villagers are so inbuilt into vanilla, but I know very little about the actual forge/coremodding/java side of modding. I've started to learn a bit; eventually I might understand enough Java to know the difference. But right now I'm still struggling with "foo implements bar()".