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.

Sign up for GitHub

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

Jump to bottom

Add ProdigyTech compatibility #173

Merged

brachy84 merged 21 commits into CleanroomMC:master from TeamDimensional:compat/prodigytech

Jul 6, 2024

Contributor

Wizzerinus commented Jun 6, 2024 •

edited

Loading

wow this is huge. I mean the mod does have a lot of things in it

I don't commonly work with Java so might have made some mistakes, the mod's recipe handling system is ridiculous (wow wee constant checks against ItemStack/OreDictIngredient because the author special cased them - I love that so much), and also some GS integrations are somewhat inconsistent (i.e. not sure what methods should be marked with @GroovyBlacklist). Otherwise it seems to work.

List of machines added: ExplosionFurnace, ExplosionFurnaceAdditives, RotaryGrinder, MagneticReassembler, HeatSawmill, OreRefinery, Solderer, PrimordialisReactor, AtomicReshaper, ZorraAltar.

Wizzerinus added 9 commits

June 6, 2024 12:34


          Made Prodigy Tech compilable, added Solderer

9bcfb77


          solderer: separated Pattern from Input

06dc275


          prodigytech: added Zorra Altar integration

e57f99d


          prodigytech: added the simple machines

108b062


          prodigytech: added primordialis reactor

59ae567


          prodigytech: added atomic reshaper


          zorra altar: better way to create custom items

316a31c


          prodigytech: added explosion furnace

31b58a4


          prodigytech: add example localization

4212f96

WaitingIdly added the mod compat label

WaitingIdly requested changes

View reviewed changes

gradle.properties Outdated Show resolved Hide resolved

src/main/resources/assets/groovyscript/lang/en_us.lang Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ZorraAltar.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnace.java Show resolved Hide resolved

...ain/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnaceAdditives.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/PrimordialisReactor.java Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Outdated Show resolved Hide resolved


          prodigytech: pull request fixes

07373e0

Contributor Author

Wizzerinus commented Jun 7, 2024

fixed the PR comments


          prodigytech: set proper @Property annotations

30f4d37

WaitingIdly requested changes

View reviewed changes

examples/preInit/prodigytech.groovy Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnace.java Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnace.java Outdated Show resolved Hide resolved

...ain/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnaceAdditives.java Outdated Show resolved Hide resolved

...ain/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnaceAdditives.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Show resolved Hide resolved

src/main/resources/assets/groovyscript/lang/en_us.lang Outdated Show resolved Hide resolved

src/main/resources/assets/groovyscript/lang/en_us.lang Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/AtomicReshaper.java Show resolved Hide resolved


          prodigytech: pr fixes (2)

b5b2d4c

Contributor Author

Wizzerinus commented Jun 7, 2024

all of these should be fixed now

thanks for being patient with a person not very experienced with java ☺️

WaitingIdly requested changes

View reviewed changes

gradle.properties Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/documentation/Documentation.java Outdated Show resolved Hide resolved

src/main/resources/assets/groovyscript/lang/en_us.lang Outdated

+              groovyscript.wiki.prodigytech.primordialis_reactor.title=Primordialis Reactor
+              groovyscript.wiki.prodigytech.primordialis_reactor.description=Turns organic matter into Primordium.
+              groovyscript.wiki.prodigytech.primordialis_reactor.addRecipe=Adds an item that can be converted.

Collaborator

WaitingIdly Jun 7, 2024

methods got renamed

groovyscript.wiki.prodigytech.primordialis_reactor.add
groovyscript.wiki.prodigytech.primordialis_reactor.remove

Contributor Author

Wizzerinus Jun 7, 2024

intended since those arent really recipes. could do addItem/ removeItem

Collaborator

WaitingIdly Jun 7, 2024

my point being that when you generate the wiki groovyscript.wiki.prodigytech.primordialis_reactor.add is the lang key checked, which doesnt exist. these lang keys should get renamed so they match the method names.

Wizzerinus added 2 commits

June 7, 2024 22:16


          prodigytech: restore debug variables to false

a7b4a92


          prodigytech: code cleanup

6b5929d

WaitingIdly requested changes

View reviewed changes

...ain/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnaceAdditives.java Outdated

+                  public boolean removeDampener(IIngredient dampener) {
+                      for (ItemStack it : dampener.getMatchingStacks()) {
+                          ExplosionFurnaceManager.Dampener externalDampener = ExplosionFurnaceManager.findDampener(it);
+                          EFAdditiveRecipe recipe = new EFAdditiveRecipe(false, dampener, externalDampener.getDampening());

Collaborator

WaitingIdly Jun 7, 2024

if ExplosionFurnaceManager.findDampener(it) doesnt find anything, it returns null, which then generates a NullPointerException on this line. this also happens to ExplosionFurnaceManager.findExplosive(it).
there should be an if (externalDampener == null) continue; to avoid this.

src/main/resources/assets/groovyscript/lang/en_us.lang Outdated

+              groovyscript.wiki.prodigytech.primordialis_reactor.title=Primordialis Reactor
+              groovyscript.wiki.prodigytech.primordialis_reactor.description=Turns organic matter into Primordium.
+              groovyscript.wiki.prodigytech.primordialis_reactor.addRecipe=Adds an item that can be converted.

Collaborator

WaitingIdly Jun 7, 2024

my point being that when you generate the wiki groovyscript.wiki.prodigytech.primordialis_reactor.add is the lang key checked, which doesnt exist. these lang keys should get renamed so they match the method names.


          prodigytech: pr fixes (3)

0ee4d0e

WaitingIdly approved these changes

View reviewed changes

brachy84 requested changes

View reviewed changes

examples/assets/placeholdername/lang/en_us.lang Show resolved Hide resolved

gradlew Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnace.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnace.java Outdated Show resolved Hide resolved

...ain/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnaceAdditives.java Outdated

Comment on lines 39 to 46

+                      for (ItemStack it : explosive.getMatchingStacks()) {
+                          ExplosionFurnaceManager.Explosive externalExplosive = ExplosionFurnaceManager.findExplosive(it);
+                          if (externalExplosive == null) continue;
+                          EFAdditiveRecipe recipe = new EFAdditiveRecipe(true, explosive, externalExplosive.getPower());
+                          remove(recipe);
+                          return true;
+                      }
+                      return false;

Member

brachy84 Jul 3, 2024

wtf???

Contributor Author

Wizzerinus Jul 3, 2024

Yeah uh, I think this might be the only sensible way to do it.

Member

brachy84 Jul 3, 2024

It isnt. if you apply the change the convo below it will become much better

...ain/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnaceAdditives.java Outdated

+                      ExplosionFurnaceManager.removeAllDampeners();
+                  }
+                  public static class EFAdditiveRecipe {

Member

brachy84 Jul 3, 2024

I dont approve how this registry is done. Do not create a custom recipe class. Rather just use the existing explosive class and add a AbstractReloadableStorage<Dampner> field. See this for example

Contributor Author

Wizzerinus Jul 3, 2024

wasn't aware of the existence of AbstractReloadableStorage

Contributor Author

Wizzerinus Jul 3, 2024

Do I not inherit from VirtualizedRegistry then? Not sure if I need to do anything if my "registry" doesn't contain recipes in itself

Contributor Author

Wizzerinus Jul 3, 2024

Ok so, I looked into the PT's code and it requires an accessor mixin. I feel like a less destructive option is better.

Member

brachy84 Jul 4, 2024

Youd still have a VirtualizedRegistry<Explossive> with a field AbstractReloadableStorage<Dampner>. And there is nothing wrong with accessor mixins.

Contributor Author

Wizzerinus Jul 4, 2024

made a version of this, unfortunately it is the only sensible version (the other version requires having a custom class too with all the consequences).

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/SimpleRecipeHandler.java Show resolved Hide resolved

...n/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/SimpleRecipeHandlerAbstract.java Outdated Show resolved Hide resolved

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/Solderer.java Outdated Show resolved Hide resolved


          prodigytech: made adjustments that made sense

ec93b1b

Contributor Author

Wizzerinus commented Jul 3, 2024

made the adjustments that made sense, not sure what to do with the other 2.

WaitingIdly reviewed

View reviewed changes

src/main/java/com/cleanroommc/groovyscript/compat/mods/prodigytech/ExplosionFurnace.java Outdated Show resolved Hide resolved

Wizzerinus added 3 commits

July 4, 2024 15:28


          prodigytech: change to 'greater than 0' wording

38c65f2


          prodigytech: somewhat improved EFAdditives code

ae5bd5e


          prodigytech: made SimpleRecipeHandlers public

Wizzerinus added 2 commits

July 4, 2024 17:41


          prodigytech: convert Explosive/Dampener into records

a2bd649


          Merge branch 'master' into compat/prodigytech

c1e6f73

# Conflicts:
#	gradle.properties
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/ModSupport.java
#	src/main/resources/assets/groovyscript/lang/en_us.lang

brachy84 approved these changes

View reviewed changes

Member

brachy84 left a comment

I will probably change 1 or 2 things once its merged.

brachy84 merged commit 3c0af9b into CleanroomMC:master

Wizzerinus deleted the compat/prodigytech branch

July 7, 2024 12:35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels