Ra1 remove

[Dream-Master]

change RA1 recipes to RA2

depends on GTNewHorizons/GT5-Unofficial#2713

[combusterf]

This refactor doesn't look like an improvement to me:

• What used to be a single line is now 5+ lines
• A lot of magic numbers are added, repeatedly. These should be in one place.
• There's no real reason why the original calls couldn't be retained as convenience methods, even if moved.

The best way forward is to add the addNnnRecipe methods to GregtechPatches that handle all the constants and the boilerplate, and perform the actual refactor by replacing GT_ModHandler references with GregtechPatches or the local equivalent

[chochem]

This refactor doesn't look like an improvement to me:

* What used to be a single line is now 5+ lines

* A lot of magic numbers are added, repeatedly. These should be in one place.

* There's no real reason why the original calls couldn't be retained as convenience methods, even if moved.

The best way forward is to add the addNnnRecipe methods to GregtechPatches that handle all the constants and the boilerplate, and perform the actual refactor by replacing GT_ModHandler references with GregtechPatches or the local equivalent

You are like a year and a half late for a discussion on how ra2 works. this is just some final cleanup of the last bits that are left over from ra1.

With the hindsight we have now I would say its been a big success. Recipes are much much easier to read and code now, there is much less magic numbers and hidden numbers, ra2 is much more consistent across machines, has better debug capabilities, etc.

[mitchej123]

• What used to be a single line is now 5+ lines

To be fair, that happens a lot with spotless 🧌

• A lot of magic numbers are added, repeatedly. These should be in one place.

I'm not sure which numbers you're referring to. The EU/t, time, etc, is now explicit instead of implicit. It makes it much easier to see at a glance what it'll do rather than being buried in a helper method.

• There's no real reason why the original calls couldn't be retained as convenience methods, even if moved.

The real reason is maintainability and given all the mods that use RA1 are under our control and maintained by us, it makes life significantly easier to purge RA1 as RA2 is superior on multiple fronts and is being driven by the people who handle most of the recipes.

The best way forward is to add the addNnnRecipe methods to GregtechPatches that handle all the constants and the boilerplate, and perform the actual refactor by replacing GT_ModHandler references with GregtechPatches or the local equivalent

I have no opinions on this, I'll leave it to Chochem/Boubou/Dream who deal with recipes far more than I do.

[combusterf]

The details on how RA works in the background do not matter here. What I see and what I find offensive to my code is the amount of code duplication: if you suddenly need 5x the space to convey exactly the same idea, you're doing it wrong IMNSHO.

[mitchej123]

if you suddenly need 5x the space to convey exactly the same idea, you're doing it wrong

I strongly disagree. Explicit > Implicit You no longer need to go look for things buried in multiple helper functions and hidden behind complicated logic. We're also talking about a diff that is +171 -43 currently. It's about maintainability and this is significantly more maintainable based on all of the input from the people responsible for maintaining recipes in the modpack.

[mitchej123]

GT_ModHandler.addExtractionRecipe(new ItemStack(ModBlocks.flower, 1, i), new ItemStack(ModItems.petal, 2, i));

Without looking, what does that do?

            GT_Values.RA.stdBuilder()
                    .itemInputs(new ItemStack(ModBlocks.flower, 1, i))
                    .itemOutputs(new ItemStack(ModItems.petal, 2, i))
                    .duration(20 * SECONDS)
                    .eut(2)
                    .addTo(extractorRecipes);

Now how about that one? Oh, I see what the input is, the output, the duration, and the eu/t. Much better code IMHO.

[boubou19]

• the need to know the signature of a method overload is an issue. RA2 solves it. Small inconvenience to write the recipe on the moment, huge win in the future.
• The magic numbers in RA1 are contained in specific parts of the recipe. So you can at least infere what it corresponds to. (even tho ideally no more magic numbers would be great)
• no need to have shit tons of overload anymore when you can just have an universal and easy way of adding recipes, so we can get rid of thousands of lines.
• Almost all of the RA1 overloads were hiding bugs, RA2 has options to debug those. We even discovered some recipes during migration because decade old recipes never actually worked.
• the 1-> 5 lines argument is a false one: what if all your inputs in RA1 makes the line ridiculously long? well spotless makes it several lines bigger.

Concerns addressed

[combusterf]

Quite funny though that on closer inspection, all the complaints about readability are about IC2 machines that have exactly one input and one output - "extract petals from flowers" calls, even if they are not formatted in builder style are quite obvious.

In the meantime, all those complex GT machine calls with multi-inputs and fluids and probabilities where all these legibility arguments actually do make sense were left completely untouched, probably because they didn't go through the IC2 wrapping stuff and therefore raised no errors.

Stuff's cleaned up to the point where you can see the context of recipes again. Have a nice day

[mitchej123]

It's also a draft WIP PR, so those haven't been touched yet I'd wager.

[boubou19]

Compliled against the RA1 removal PR locally, it builds now.

[combusterf]

Some recipes appear to have been dropped

adressed

[boubou19]

Some recipes appear to have been dropped

Which ones?

[Dream-Master]

i see no recipes are droped