-
Notifications
You must be signed in to change notification settings - Fork 29
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
add Additional Enchanted Miner compat #277
base: master
Are you sure you want to change the base?
Conversation
additionalenchantedminer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs spotless to be applied. is using the scala classes needed? WorkbenchPlusRecipe
doesnt seem needed, can just call WorkbenchRecipe
methods directly.
src/main/java/com/cleanroommc/groovyscript/compat/mods/ModSupport.java
Outdated
Show resolved
Hide resolved
import scala.Option; | ||
import scala.collection.JavaConversions; | ||
import scala.collection.Map; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have to use scala here?
...in/java/com/cleanroommc/groovyscript/compat/mods/additionalenchantedminer/WorkBenchPlus.java
Outdated
Show resolved
Hide resolved
...in/java/com/cleanroommc/groovyscript/compat/mods/additionalenchantedminer/WorkBenchPlus.java
Outdated
Show resolved
Hide resolved
...in/java/com/cleanroommc/groovyscript/compat/mods/additionalenchantedminer/WorkBenchPlus.java
Outdated
Show resolved
Hide resolved
...in/java/com/cleanroommc/groovyscript/compat/mods/additionalenchantedminer/WorkBenchPlus.java
Outdated
Show resolved
Hide resolved
|
||
|
||
public static WorkbenchPlusRecipe addRecipe(WorkbenchPlusRecipe recipe) { | ||
addIngredientRecipe(recipe.location, recipe.output, recipe.energy, recipe.input, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not WorkbenchRecipe.addIngredientRecipe
instead of importing a static just for this
} | ||
|
||
@RecipeBuilderDescription(example = | ||
@Example(".output(item('minecraft:nether_star')).input(item('minecraft:diamond'),item('minecraft:gold_ingot')).energy(10000)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you provide a second example?
return WorkbenchPlusRecipe.removeByOutput(output); | ||
} | ||
|
||
public Boolean removeById(String id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be used by people using GroovyScript be calling this method? if so, use @MethodDescription
and state what it does. otherwise, private or @GroovyBlacklist
. same with removeAll
.
|
||
import static com.yogpc.qp.recipe.WorkbenchRecipe.addIngredientRecipe; | ||
|
||
public class WorkbenchPlusRecipe { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the point of this class? what purpose does it serve that it couldnt be replaced by some code in the register()
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I think about it, I realize that I don't need WorkbenchPlusRecipe, I'll fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing changed here.
…nalenchantedminer/WorkBenchPlus.java Co-authored-by: Waiting Idly <[email protected]>
Additionalenchantedminer
Fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still need to apply spotless.
// groovyscript.wiki.quarryplus.work_bench_plus.title: | ||
// groovyscript.wiki.quarryplus.work_bench_plus.description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where lang? peep the groovy.log
to see the missing lang keys and add a translation for those
@Override | ||
public void onReload() { | ||
removeScripted().forEach(recipe -> WorkbenchRecipe.removeRecipe(recipe.getLocation())); | ||
restoreFromBackup().forEach(ModSupport.ADDITIONAL_ENCHANTED_MINER.get().WorkBenchPlus::add); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
calling this method adds the method to the scripted list, which should not be done.
import com.cleanroommc.groovyscript.compat.mods.GroovyPropertyContainer; | ||
|
||
public class AdditionalEnchantedMiner extends GroovyPropertyContainer { | ||
public final WorkBenchPlus WorkBenchPlus = new WorkBenchPlus(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fields should be in camelCase
- WorkBenchPlus
is PascalCase, it should be workBenchPlus
|
||
import static com.yogpc.qp.recipe.WorkbenchRecipe.addIngredientRecipe; | ||
|
||
public class WorkbenchPlusRecipe { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing changed here.
add compat with Additional Enchanted Miner
WorkBench