Add Tinker Complement's High Oven support

[Riernar]

Hi @jaredlll08 ,

I added @KnightMiner 's tcomplement support to ModTweaker, as asked for in #718 . I tried keeping the same code style as other tcomplement support and used a new package to organise code.

@KnightMiner , here's the PR. I hope I'm using your API correctly - let me know if anything is amiss !

[Riernar]

For testing purposes, here is a small script that uses just CraftTweaker, ModTweaker, Mantle-TinkerConstruct-Tcomplement. It requires a tcomplement recompiled from source though, as a change was needed in the source code to implement the compatibility.

import mods.tcomplement.highoven.HighOven;
import mods.tcomplement.highoven.IMixRecipe;

HighOven.removeFuel(<minecraft:coal:1>);
HighOven.addFuel(<minecraft:diamond>, 1000, 25);

HighOven.removeHeatRecipe(<liquid:steam>);
HighOven.addHeatRecipe(<liquid:lava> * 100, <liquid:water> * 1000, 450);

HighOven.removeMixRecipe(<liquid:steel>);
HighOven.newMixRecipe(<liquid:cobalt> * 144, <liquid:iron> * 144, 1000)
    .addOxidizer(<minecraft:stone>, 25)
    .addReducer(<minecraft:cobblestone>, 35)
    .addPurifier(<minecraft:redstone>, 45)
    .addPurifier(<ore:stickWood>, 100)
    .register();

[KnightMiner]

You should not need to compile from the source for this to work, build 0.4.2.39 has the change from your PR in it which can be pulled from Maven

[Riernar]

Hi @KnightMiner ,

Happy to help ! I needed that for my own pack, so its not completely selfless. Your code interface is great, it easy to hook into it and add/remove stuff. And from a pack making point of view, your recipes are very versatile and offers a tons of possibilities for e.g. metallurgy, which is awesome ! No worries about the interface :)

I integrated your corrections, shamelessly copy-pasting your IIngredient RecipeMatch wrapper, and added a class for manipulating already existing mix recipes (including scripted ones).

There is now a MixRecipeBuilder (the old MixRecipeHelper / mods.tcomplement.highoven.IMixRecipe) and a MixRecipeManager. The interface of the former is as before, while the manager allows to edit other mix recipes, including scripted ones.

I'm not sure I'm checking the matching of RecipeMatch for additive removal in the right order, could you confirm that ? I want it to cancel the event if the added additive matches the IIngredient in any way. null can be used to remove all additives of a type.

Anyway, thanks a lot for your help, the code is much cleaner and safer now !

PS. I should really learn a bit about maven and gradle at some point, thanks for pointing out jars are available from the maven

[Riernar]

@KnightMiner

Am I matching heat recipe, mix recipes and additives removals correctly ? I'd like that you don't have to specify the exact amount in zenscript for the removal to work.

[KnightMiner]

I have not had a chance to look into it too in-depth, but I think it looks right for the fluids. There's a better way to handle the item ingredient matching I think, I'll leave some comments on that tonight when I have some free time

[Riernar]

@KnightMiner I'd like your input on that, especially for the MixRecipeManager. I get a RecipeMatch from storing the user's instructions and one from the event, and I don't know how to test if the user's one includes the event's one - previous code throws CannotCast as the List<ItemStack> cannot be casted to NonNullList<ItemStack>.

[Riernar]

I think this PR is ready for merging. Everything seems to work properly according to the test script and all use case we could think of have been covered.

[KnightMiner]

Test script needs a manageMixRexipe test since you added that method after the script. There is a place in the CT source code for you to commit they script for later by the way, so it's not just in this PR

[Riernar]

I did add a test script testing all methods, see my latest commit

I think I haven't forgot anything in it

[Riernar]

@KnightMiner I added your last suggestion (pairs). Do you see anything else amiss I should change, or are we ready for merging ?

[KnightMiner]

Ah, did not notice the committed one, it did not show up on the changed files yesterday so I missed it.

I suggest you change one of the two manageMixRecipe calls to use a recipe that exists by default such as pig iron just so we know it works on base recipes. Doubt it would break there, but good to test anyways. Edit: nevermind, I see the pig iron manage there.

Otherwise, I think it looks good. I still want to test the latest stuff in game before I give my full approval, but as far as code goes it looks good

[Riernar]

Maybe it didn't show up because it's a new file ? Anyway, I'm glad everything works properly. I imagine the bug in TComplement's is due to internal code and doesn't affect the API. I must admit I did not check if the recipes actually worked and just trusted JEI ^^

Thanks a lot for your extensive help and advices on that compatibility, it's much better thanks to you ! Plus it's quite awesome to work with a mod author !

[Riernar]

Hi @jaredlll08 We are ready for merging this support in ModTweaker ! I left the version handling to you, so you can keep everything consistent. There might be a slight merge conflict with #729 since I copy-pasted the mantle helper. The file are the same, I hope it's not too troublesome. If you need anything else for that PR, feel free to ask !

[Riernar]

@KnightMiner I guess I forgot High Oven's melting overrides as requested in #541 , those seems to be supported by TCompRegistry. I'll add a method in a bit

[Riernar]

@KnightMiner I added support for High Oven melting overrides as in #541 , this was pretty much a copy-past from the Override class.

@jaredlll08 Everything should be good now

[Riernar]

Hi @jaredlll08 , Do you know when you'll have time to review and eventually merge ? I know it can be pretty tedious, so if I can help you in any way, please ask !

[jaredlll08]

No idea. I am currently dealing with university projects, when that has calmed down, I will go through and look at all of modtweaker related stuff.

(even if I merged it now, it would still be a bit before a release)

[Riernar]

No problem, I'll just keep an eye to be around when/if you need me. Merge/release the way you think is best - no need for hasty merges while you're dealing with more important stuff. Good luck with your projects !

(Btw, I won't be much available in August, if that matters)

[hoklai1997]

@jaredlll08 , do you think you'll have the chance to merge this soon? I was hoping to use the integration in a modpack, but if you don't think you'll get to merging the tcomplement support, then I can look at other options. Don't mean to rush you, just curious about a timeline, Thanks!

[kindlich]

Hey @Riernar

short question since I only now realized that I merged the wiki PR already lol: Since the original PR was filed you have added some more commits to this PR. Does this affect the ZS classes, as in, is the wiki page you wrote back then still up to date with the commits you have added later?

[Riernar]

Hi @kindlich , Just had a look to make sure, I think the documentation is up to date. The last feature I added was High Oven Overrides' modifications, and the associated commit was pushed on the PR before the merge. The md formatting is broken here and there, though, and some sentences are not very clear. I guess I should fix that at some point.

[Riernar]

Hi @hoklai1997 , Since ModTweaker is released under the MIT license, I think you can use and distribute a jar built from my fork, in the mean time (don't quote me on that though, I'm no legal expert). That's just a few gradle commands, if you need help feel free to ask.

If you do, please report any bug in the high oven compatibility, so I can fix them. Testing in an actual modpack might reveal some corner cases. And of course, don't forget to credit Jared ;)

[hoklai1997]

Thanks, sounds like things might be moving, so I think I’ll wait for the official release. However, I do have plenty of scripts using your compatibility, so I’ll definitely let you know if I find anything!

[Fr4gtastic]

Hello @jaredlll08, Not trying to rush you or anything, but can we expect this PR to be merged anytime soon? If not, could the docs entry at least be updated to mention the fact that this feature is not yet implemented? It's a bit confusing at the moment. Thanks!

[hoklai1997]

Hey @Riernar, I did decide to build the jar for now, at least so I can continue development, I'll let you know if I run into anything! I'm hoping @jaredlll08 will get the chance to merge it officially before my release, but if not I'll checkin again.