Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing the Chemistry, Take Two #1492

Merged
merged 17 commits into from
Mar 22, 2021
Merged

Conversation

serenibyss
Copy link
Collaborator

@serenibyss serenibyss commented Feb 21, 2021

Introduction

This PR is a continuation of #1414, addressing a few dangling issues, adding some new Electrolysis, and a few more recipes that help make our chemistry more robust. Additionally, I included a major refactor of our ChemistryRecipes file to make it much easier to read and edit later on down the road.

The Refactor

Here is an example of a newly refactored chemistry recipe:

CHEMICAL_RECIPES.recipeBuilder()
            .notConsumable(new IntCircuitIngredient(2))
            .input(dust, Silicon)
            .fluidInputs(Water.getFluid(1000))
            .fluidInputs(Chlorine.getFluid(4000))
            .fluidInputs(Methane.getFluid(2000))
            .output(dust, Polydimethylsiloxane, 3)
            .fluidOutputs(HydrochloricAcid.getFluid(2000))
            .fluidOutputs(DilutedHydrochloricAcid.getFluid(2000))
            .duration(480).EUt(96).buildAndRegister();

The method calls are organized as follows:

[circuit, input(s), fluid input(s), output(s), fluid output(s), duration, EUt, buildAndRegister]

Additionally, as you can see from the output line, I added a couple methods into RecipeBuilder to allow for the same syntax on outputs that we could already do in input(). With this change, we do not need to call outputs(OreDictUnifier.get(dust, Carbon, 2)), and instead just output(dust, Carbon, 2).

I also separated out multiline additions into their own lines for readability. When doing this, I made sure to maintain the original ordering. In some cases, this shouldn't matter, but in others (Distillation Tower), it is very important.

Old Format:

.fluidInputs(<fluid1>.getFluid(x), <fluid2>.getFluid(y), <fluid3>.getFluid(z)

New Format:

.fluidInputs(<fluid1>.getFluid(x))
.fluidInputs(<fluid2>.getFluid(y))
.fluidInputs(<fluid3>.getFluid(z))

This made some files very long, so I separated our ChemistryRecipes file out, having a new one called ReactorRecipes that contains all the Chemical Reactor recipes previously in ChemistryRecipes.

Additionally I added several static imports to remove some unnecessary verbosity such as MaterialIconSet.DULL, Materials.Carbon, RecipeMaps.CHEMICAL_REACTOR to keep the code as clean as possible. I also pulled the recipes from MachineRecipeLoader#registerChemicalRecipes() and put them into ChemistryRecipes and ReactorRecipes to keep everything together.

Chemistry Fixes

There were a total of 6 outstanding issues remaining that I did not address in #1414. They are all quite minor, either adjusting amounts, or changing an output material.

  • Dichlorobenzene: The most significant recipe issue remaining. The old recipe:
    0.5C₆H₆ + 2Cl -> HCl + 0.5C₆H₄Cl₂
    This recipe lost 0.5B of Chlorine on each reaction. The new recipe:
    C₆H₆ + 4Cl -> 2HCl + C₆H₄Cl₂
  • Tetrafluoroethylene: A small adjustment. Used half-mols, and also output Dilute Hydrochloric Acid instead of normal.
    Old: 2HF + CHCl₃ -> 3HCl(dil.) + 0.5C₂F₄
    New: 4HF + 2CHCl₃ -> 6HCl + C₂F₄
  • Hypochlorous Acid: Output Dilute Hydrochloric Acid instead of normal, like above.
    Old: 2Cl + H₂O -> HCl(dil.) + HClO
    New: 2Cl + H₂O -> HCl + HClO
  • Phosphoric Acid: This recipe was problematic (using half-mols), but I also believe this recipe should be removed. It is very nonsensical, but since it is the easiest way of producing the acid, it is probably best to leave it.
    Old: P + 2.5O + 1.5H₂O -> H₃PO₄
    New: 2P + 5O + 3H₂O -> 2H₃PO₄
  • Nitration Mixture: Since Nitration Mixture is a mixture and not a different chemical, I changed its recipes to produce 2B of Nitration Mixture instead of 1, to respect its volume (similar to something like Drilling Fluid). I also changed its separation recipe to be this same way, and moved it to a Centrifuge recipe instead of an Electrolyzer recipe.
    Old: 1 Sulfuric + 1 Nitric -> 1 Nitration Mixture, 1 Nitration Mixture -> (electrolyzer) 1 Sulfuric + 1 Nitric
    New: 1 Sulfuric + 1 Nitric -> 2 Nitration Mixture, 2 Nitration Mixture -> (centrifuge) 1 Sulfuric + 1 Nitric
    This is a precedent being set by our chemistry, that I have already been incorporating in similar mixtures in Gregicality, as they have others on top of this (like Aqua Regia).
  • Zeolite: I removed the Zeolite electrolysis recipe, by simply setting DISABLE_DECOMPOSITION. In base GTCE, this recipe was impossible, since it took well over a stack of material to electrolyze it. It does not make sense to me to have this recipe enabled when it is entirely impossible to do in the mod by itself. There was an (ancient) issue Zeolite Dust Electrolyzing Recipe has input over ItemStack size #354 that said this recipe will only be possible in a multiblock Electrolyzer, and I think at this point it is safe to say we have no intention to add this multiblock to GTCE. As a result, I will re-enable electrolysis for it in Gregicality since they have the Large Electrolyzer multiblock.

Chemistry Additions

I re-enabled electrolysis of some organic chemicals (much to the chagrin of many chemists) to match how the others were already handled. I added a few special recipes though, not using the standard generated electrolysis format.

Custom Electrolysis Recipes:

  • Acetic Acid: Based on the Kolbe Process, I made the Acetic Acid electrolysis recipe:
    2CH₃COOH -> H₃CCH₃ + 2CO₂ + 2H

These next three are based loosely on realistic chemical processes, more accurate than straight decomposition:

  • Chloromethane: 2CH₃Cl -> H₃CCH₃ + 2Cl (creates Ethane)
  • Methyl Acetate: NaOH(catalyst) + CH₃COOCH₃ + H₂O -> CH₃COOH + CH₃OH (creates Acetic Acid and Methanol)
  • Acetone: 2C₃H₆O -> C₃H₈ + 3C + 2H₂O (creates Propane)

New Chemical Reactor Recipes:

  • Sodium Bisulfate Use: When researching Sodium Bisulfate's decomposition, I found a reaction with Water to produce Sodium Hydroxide and Sulfuric Acid:
    NaHSO₄ + H₂O -> NaOH + H₂SO₄
    With this recipe, we now have a loop with its formation recipe:
    NaCl + H₂SO₄ -> HCl + NaHSO₄
    As a result, given Water and Salt, we can transform them into Sodium Hydroxide and Hydrochloric Acid, giving them a new and interesting production method.
  • Phenol: There are two new recipes to create Phenol, on top of the existing Cumene Process recipe.
    • Benzene Method: Using the theoretical Oxidation of Benzene:
      C₆H₆ + O -> C₆H₅OH
      This recipe is theoretically possible, though has not yet been done in a real-world situation. As a result, I made this recipe EV Tier to reflect its chemical difficulty. Unfortunately I was not able to use the Oxidation of Toluene recipe since it has too many fluid outputs to put in a Chemical Reactor.
    • Dichlorobenzene Decomposition: Using a modified Raschig-Hooker Process, I added the recipe:
      C₆H₄Cl₂ + 2NaOH -> C₆H₅OH + 2NaCl + O
      I modified it from the standard process since it uses Chlorobenzene and not Dichlorobenzene, which we do not have in GTCE.
  • Ethylene: Inspired by a new research paper on creating Hydrocarbon chemicals in an eco-friendly way, I added a recipe to create Ethylene from Glycerol:
    C₃H₈O₃ + CO₂ -> 2C₂H₄ + 5O
    This recipe is quite significant in terms of creating Polyethylene, so I made it HV, meaning that when the player creates mass amounts of Glycerol for Epoxy, they can now also use it for Ethylene.

Outcome

Compatibility

This PR has similar potential compatibility issues as #1414, though this time only 5 existing recipes were modified, so its impact will be a lot less.

serenibyss and others added 4 commits February 19, 2021 20:56
Due to @DStrand1 's pull request here(GregTechCE#1414), the recipes for processing Uraninite into Uranium 238 were changed.  One of the recipes, which outputs 144mb of liquid Aluminium, allows you to delay making a blast furnace until further materials are needed.
By changing the EU/t cost of the recipe to require MV power, it ensures that you need to make your first aluminium from smelting in the blast furnace.

In the future, uraninite processing needs to be looked at in more depth.
@serenibyss
Copy link
Collaborator Author

serenibyss commented Feb 21, 2021

The last commit addressed an issue where Uraninite processing used an Aluminium Dust, and output 144mB Aluminium to conserve the material. However, since this was an LV recipe, it was possible to skip the EBF and simply do this reaction then Fluid Solidify the Aluminium. This recipe has been moved to MV to avoid this, and the duration was cut to incorporate the overclocking speed increase.

If we implement the Uranium Processing changes discussed in #1426, this issue would no longer be present and can be resolved more effectively.

Copy link
Member

@LAGIdiot LAGIdiot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have commend your bravery touching this area again.
I will leave checking if chemistry is correct/makes sense to community.
I like your idea to extract recipes to own files but would not be easier to have it machine per file?

@LAGIdiot LAGIdiot added release note needed rsr: minor Release size requirements: Minor type: bug Something isn't working type: feature New feature or request open for discussion labels Feb 22, 2021
@serenibyss
Copy link
Collaborator Author

serenibyss commented Feb 22, 2021

I like your idea to extract recipes to own files but would not be easier to have it machine per file?

I think this would make the most sense, but I was not sure if this was the path you wanted to take. I can create a subdirectory in recipes “chemistry” and create files for each machine (that has enough recipes to justify it, I will not make a file for 2 fluid heater recipes :) ).

I have a few more things to add to this PR, and a few discussion points that I will post here when I can, but I’ve got things to do right now and am trying to just get a quick response down. As a result, I will temporarily convert this PR to draft.

@serenibyss serenibyss marked this pull request as draft February 22, 2021 18:52
@LAGIdiot
Copy link
Member

I like your idea to extract recipes to own files but would not be easier to have it machine per file?

I think this would make the most sense, but I was not sure if this was the path you wanted to take. I can create a subdirectory in recipes “chemistry” and create files for each machine (that has enough recipes to justify it, I will not make a file for 2 fluid heater recipes :) ).

Sure go for it. I would rather not have own files for recipe maps with less then 5 recipes.

@serenibyss
Copy link
Collaborator Author

serenibyss commented Feb 23, 2021

With this above refactor, I separated the chemistry recipes into more files, as discussed above. I also added a few more methods to RecipeMap to avoid having to use ItemStack in recipe generation. An example:
OLD: .inputs(new ItemStack(Items.PAPER, 1, OreDictionary.WILDCARD_VALUE))
NEW: .input(Items.Paper, 1, true)

Supports blocks and items, meta value, and wildcard (input only).

@serenibyss
Copy link
Collaborator Author

There was a recipe conflict with Hydrogen Sulfide in the Chemical Reactor:
H2S + 3O -> H2O + SO2
H2S + 4O -> H2SO4

As you can see, these share identical inputs with only varying amounts. I added circuits to both of these recipes, of values 1 and 2 respectively.

@serenibyss serenibyss marked this pull request as ready for review February 23, 2021 21:29
@serenibyss
Copy link
Collaborator Author

The last commit addressed an issue where Uraninite processing used an Aluminium Dust, and output 144mB Aluminium to conserve the material. However, since this was an LV recipe, it was possible to skip the EBF and simply do this reaction then Fluid Solidify the Aluminium. This recipe has been moved to MV to avoid this, and the duration was cut to incorporate the overclocking speed increase.

If we implement the Uranium Processing changes discussed in #1426, this issue would no longer be present and can be resolved more effectively.

I want to really highlight this issue and make a case for implementing the Uraninite processing in #1426 soon, to avoid further issues.

Before this PR, the EBF was entirely optional due to an exploit with Uraninite and Aluminium that output fluid Aluminium at LV-voltage. Even with this PR, the EBF is again required for an MV chemical reactor, but can easily be ignored and use this method instead, for less power usage and recipe time. This recipe is extremely problematic, and I think it should be either entirely removed, or we should add better Uraninite processing.

I want to mention that there is the other recipe, outputting fluid Magnesium, which while not progression-breaking, is still a very bad recipe in my opinion.

@serenibyss
Copy link
Collaborator Author

serenibyss commented Feb 28, 2021

In this last commit, I fixed Yttrium Barium Cuprate Oxide and Tungstencarbide.

YBCO:
OLD: Y + 2Ba + 3Cu -> 6 YBa2Cu3O7 dust
You can see here that oxygen was being created from nothing. This was a crafting table recipe, and is now a mixer recipe with oxygen. I also increased its recipe output to match its electrolysis recipe.
NEW: Y + 2Ba + 3Cu + 7O -> 13 YBa2Cu3O7 dust

Tungstencarbide:
I changed this to match Tungstensteel, which was Steel + Tungsten -> 2 Tungstensteel + 1 small dark ashes
NEW: Carbon Dust + Tungsten -> 2 Tungstencarbide + 1 small dark ashes

This YBCO issue is definitely needed, since currently there is an Oxygen dupe bug present. The Tungstencarbide change may warrant further discussion, as there is inconsistencies as to how to handle this situation. The two arguments:

  • Red Alloy Argument
    Red Alloy, as we all know, is Copper + 4 Redstone -> 1 Red Alloy. With this as a precedent, it makes sense that Tungstencarbide is similarly one ingot output

  • Electrolysis Argument
    Tungstencarbide Dust's electrolysis recipe is currently 2 Tungstencarbide Dust -> 1 Tungsten + 1 Carbon. With the old recipe remaining, you are losing an ingot of Tungsten on each electrolysis (Carbon was balanced, as it used to output 2 Small Ash, which is effectively 0.5 Carbon each recipe).

@LAGIdiot
Copy link
Member

LAGIdiot commented Mar 7, 2021

From codding perspective this looks good. I have bit of reservation regarding nonfunctional change to Materials but I am not gonna pursue it.

From chemical stand point, well I will wait for some community members to speak up.

@serenibyss
Copy link
Collaborator Author

I changed two more things to address #1525:

  • Titanium Tetrachloride was declared as TiC₂Cl₂, which was easily ignored in the past, but with fluid tooltips, this needs to be changed. Updated to the proper formula TiCl₄
  • Nickel-Zinc Ferrite was changed slightly. Its old recipe was:
    1 ferrite mixture dust + 2O -> 1 nickel-zinc ferrite ingot
    With NZF having the formula, NiZnFe₄O₈, the oxidation was wrong. I decided in an attempt to keep the balance of the material, to change the oxidation to 1.5B of Oxygen. The proper chemical recipe would be:

NiZnFe₄ + 8O -> NiZnFe₄O₈ which corresponds to 6 mixture dust + 8B O -> 14NZF dust. This didnt feel ideal to me, as it was a large balance change. If you divide the 8 oxygen by 6 dust, you get 1.3333B O, which would be the amount of oxygen per dust to fully oxidize it. I decided to make the recipe take 1.5B so that it is an even number, and only a slight excess, which I think is fine, unless we are okay with a big balance change.

@Exaxxion
Copy link
Collaborator

The chemistry changes (up to the comments asking for feedback) all look good to me. I'll weigh in on the feedback requests:

We had talked a bit about YBCO in Discord, but to recap I think the electrolysis argument is more persuasive there. This recipe is rather contrived as the real recipe for this stuff involves several chemicals that are not in GTCE, but otherwise is consistent with our chemistry rules expectations.

The new behavior for Tungstencarbide and the change to the material definition for TiCl4 both make sense to me. The former is now consistent with other alloys, and the latter was clearly defined incorrectly. The recipe to make TiCl4 does involve Carbon as a reactant, but it definitely doesn't end up in that fluid.

Given that there are no particularly tricky or rare materials in NZF, I don't think there's too substantial of a balance implication in giving it an accurate number of dusts for its formula (also considering that we basically did this for YBCO anyway). If it's possible to decompose this stuff, the ratio of the proposed recipe using an approximated half-mole of oxygen would lead to an imbalance, and it deviates from the rest of our chemistry overhauling that actively avoids doing partial moles.

@LAGIdiot
Copy link
Member

I will mark this as approved. But if there is need for change or addition then just do it. But if possible I would like to get it out on next release.

@serenibyss
Copy link
Collaborator Author

serenibyss commented Mar 18, 2021

In these last 5 commits, I finished up some last things discussed here and on Discord:

  • Fix Phosphorus recipe conflict: There was a conflict in the recipes for Phosphoric Acid using Phosphorus, and Phosphorus Pentoxide, so these recipes have gained circuits. It should only be necessary for Pentoxide, but I added it to both to be safe

  • Finish NZF: I changed the recipe to match with YBCO, now being (Ni + Zn + 4Fe) + 8O -> 14 NZF Ingot (1200K, EBF) where the Ni + Zn + 4Fe is ferrite mixture. I increased its duration to account for the increased amount processed per recipe.

  • Finish Uraninite: I adjusted the mole amounts to be consistent, and changed its EUt and duration to match if the old LV recipe were overclocked to MV, to it now requires MV but is otherwise exactly the same EUt and duration.

  • Normalize Red Alloy: This is a relatively big change. I added EBF recipes to Red Alloy, taking 1 Copper (ingot or dust) and 1 Redstone Dust to make 2 Red Alloy Ingots. This makes Red Alloy cheaper post-EBF, and makes the early game Alloy Smelter recipe a "lossy early-game" recipe like many other early game recipes. The result of this is that Red Alloy now follows proper mole amounts. Additionally, I changed its Material Composition to be only 1 Copper and 1 Redstone so that its electrolysis is not a positive loop. As a result, Annealed Copper had to gain a circuit to avoid a recipe conflict.

  • Fix TNM: Tetranitromethane's recipe was still bad on a further inspection.
    OLD: 4CH₃COOH + 4C₂H₂O + 4HNO₃ -> C(NO₂)₄ + 7CH₃COOH. This recipe had a lot of problems, including having the same fluid in and out.
    NEW: 2CH₃CO₂CH₃ + 4HNO₃ -> C(NO₂)₄ + 8H₂O + 5C. I would have liked to do better than this, but this was as good as I could do with the limited output slots/pre-existing materials.

With these last few changes, this PR is finished and ready for merge.

@Exaxxion
Copy link
Collaborator

Unfortunate that the TNM recipe from before turned out to be imbalanced, good catch. The new one seems reasonable enough given our limitations. I agree the PR seems ready to go.

Outside the scope of this PR, but it would be really helpful if at some point we could add a few more chemicals, as it would allow us to implement the actual processes rather than rough approximations that are harder to balance and often need circuits to prevent conflicts.

@Exaxxion
Copy link
Collaborator

Just this morning we got an issue reported regarding a recipe overlap between Invar and Ferrite Mixture in the mixer. While this PR is correcting the formula for NZF, It would probably be a good idea to also disambiguate these recipes, as Invar is a subset of the ingredients of Ferrite Mixture.

@serenibyss
Copy link
Collaborator Author

serenibyss commented Mar 21, 2021

Lastly, I removed electrolysis recipes for 5 materials:

  • Polyethylene
  • Epoxy
  • Silicone Rubber
  • Polycaprolactam
  • Polytetrafluoroethylene

All 5 of these had electrolysis recipes as dusts only, and since the chemistry for polymerized plastics in GTCE is a little... inaccurate due to the transition from dust to ingot moles, these were removed for safety. I doubt many people even knew they existed anyway.

@LAGIdiot LAGIdiot merged commit 57b0e39 into GregTechCE:master Mar 22, 2021
@serenibyss serenibyss deleted the chemfixtwo branch March 24, 2021 18:48
Tictim referenced this pull request in Tictim/GregTech Feb 14, 2023
* update advanced item/fluid detectors' tooltip

* small changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open for discussion release note needed rsr: minor Release size requirements: Minor status: accepted type: bug Something isn't working type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] some chemical stuff
4 participants