-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
Recipe Support #7150
base: dev/feature
Are you sure you want to change the base?
Recipe Support #7150
Conversation
# Conflicts: # src/main/java/ch/njol/skript/classes/data/BukkitClasses.java # src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java # src/main/java/ch/njol/skript/classes/data/DefaultComparators.java # src/main/resources/lang/default.lang
This comment was marked as resolved.
This comment was marked as resolved.
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.
So uuh thinking about moving this to a module?
nada |
src/main/java/ch/njol/skript/conditions/CondDiscoveredRecipes.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprRecipeCookingTime.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprRecipeCookingTime.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprRecipeCookingTime.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprRecipeCookingTime.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprRecipeExperience.java
Outdated
Show resolved
Hide resolved
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.
I decided more changes were needed, we're aiming for 100
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/SecRegisterRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeModule.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
I mentioned it to smurf once, but mentioning it in the pr itself will be good, what's the thought about a proper RecipeChoice implementation to have more support of ingredients. If this goes through a custom ItemTypeChoice should be created to enable skript's item types correctly and not converting to an itemstack always. This should be pretty safe due to the fact recipes and these can't be serialized by default |
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.
partial review
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeWrapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/EffDiscoverRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/EffDiscoverRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/EffDiscoverRecipe.java
Outdated
Show resolved
Hide resolved
So, I tried to have the subclasses extends their respective Bukkit recipe class, but unfortunately it wont let me. |
ah are they final? |
It just says "Class can not extend multiple classes" |
oh right the sub recipes aren't interfaces |
src/main/java/org/skriptlang/skript/bukkit/recipes/RegisterRecipeEvent.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/EffDiscoverRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/EffDiscoverRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/EvtDiscoverRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprAllRecipes.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeCategory.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeCategory.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeCategory.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/SecRegisterRecipe.java
Outdated
Show resolved
Hide resolved
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.
on the way to most commented pr 💪
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeResult.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeGroup.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeExperience.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeCookingTime.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/MutableRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RecipeCategory.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/SecRegisterRecipe.java
Outdated
Show resolved
Hide resolved
# Conflicts: # src/main/java/ch/njol/skript/Skript.java # src/main/resources/lang/default.lang
src/main/java/org/skriptlang/skript/bukkit/recipes/MutableRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/MutableRecipe.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/RegisterRecipeEvent.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/CondDiscoveredRecipes.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/CondDiscoveredRecipes.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeIngredients.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeIngredients.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeIngredients.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeKey.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/ExprRecipeResult.java
Outdated
Show resolved
Hide resolved
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.
Can we possibly implement the transmute recipe type? https://jd.papermc.io/paper/1.21.4/org/bukkit/inventory/TransmuteRecipe.html
src/main/java/org/skriptlang/skript/bukkit/recipes/MutableRecipe.java
Outdated
Show resolved
Hide resolved
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.
looking pretty good
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/CondSmithingData.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/recipes/elements/CondSmithingData.java
Show resolved
Hide resolved
}) | ||
@RequiredPlugins("Paper") | ||
@Since("INSERT VERSION") | ||
|
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.
"NOTES:", | ||
"This condition can only be used with Smithing Recipes on PaperMC version 1.19", | ||
"This condition can be used with Smithing Transform and Smithing Trim Recipes on PaperMC version 1.20.0 -> 1.20.4", | ||
"This condition is redundant for PaperMC versions 1.20.5+ due to the removal in favor of copying data components and will always return 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.
is there a reason for picking always true instead of always false or just not registering?
I'm also not sure why this isn't merged with data component condition, because I imagine most users consider components to be nbt.
"\tset the recipe ingredients to diamond, air, diamond, air, emerald, air, diamond, air and diamond", | ||
"\t#OR", | ||
"\tset the recipe ingredients of the 1st row to diamond, air and diamond", | ||
"\tset the recipe ingredients of second row to air, emerald and air", | ||
"\tset the recipe ingredients of the third row to diamond, air and diamond", | ||
"\tset the recipe result to beacon", | ||
"", | ||
"set {_recipe} to a shapeless recipe with id \"my_recipe\":", | ||
"\tset recipe ingredients to iron ingot, gold ingot, iron ingot, nether star, 5 obsidian, nether star, iron ingot, gold ingot and iron ingot", | ||
"\tset recipe resulting item to beacon named \"OP Beacon\"", | ||
"", | ||
"set {_recipe} to new blasting recipe with the id \"my_recipe\":", | ||
"\tset the recipe input item to netherite ingot named \"Impure Netherite\"", | ||
"\tset the recipe result item to netherite ingot named \"Pure Netherite\"", | ||
"", | ||
"set {_recipe} to a new smithing transform recipe with key \"my_recipe\":", | ||
"\tset the recipe base item to diamond helmet", | ||
"\tset the recipe template item to paper named \"Blueprint\"", | ||
"\tset the recipe addition item to netherite ingot named \"Pure Netherite\"", | ||
"\tset the recipe result to netherite helmet named \"Pure Helmet\"", | ||
"", | ||
"set {_recipe} to a new transmute recipe with key \"my_recipe\":", | ||
"\tset the recipe input item to leather helmet", | ||
"\tset the recipe transmute item to nether star named \"Free Upgrade\"", | ||
"\tset the recipe result to netherite helmet" |
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.
i know it's probably already been discussed but do you think there's any safe way to get rid of all the recipe
keywords here? It feels awkward to have to repeat them
@Override | ||
@SuppressWarnings("unchecked") | ||
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { | ||
selectedPattern = recipePatterns[(int) Math.floor((double) matchedPattern / 2)]; |
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.
selectedPattern = recipePatterns[(int) Math.floor((double) matchedPattern / 2)]; | |
selectedPattern = recipePatterns[matchedPattern / 2]; |
integer division should truncate for you
if (getParser().isCurrentEvent(CreateRecipeEvent.class)) { | ||
boolean classFound = false; | ||
for (Class<? extends Event> eventClass : selectedPattern.eventClasses) { | ||
if (getParser().isCurrentEvent(eventClass)) { |
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.
You should be able to use the varargs version of isCurrentEvent to check multiple at once
@Override | ||
protected ItemStack @Nullable [] get(Event event, Recipe[] source) { | ||
if (isEvent) | ||
return null; |
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.
Can we not get the ingredient of an in-process recipe? I can see it being somewhat useful:
set ingredients to func()
if ingredients contains xyz:
assert {_get} is set with "Transmute recipe was not added to the server" | ||
|
||
assert the recipe type of {_recipe} is transmute recipe with "Created recipe is not a transmute recipe" | ||
assert the recipe type of {_recipe} is transmute recipe with "Retrieved recipe is not a transmute recipe" |
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.
assert the recipe type of {_recipe} is transmute recipe with "Retrieved recipe is not a transmute recipe" | |
assert the recipe type of {_recipe} is transmute recipe with "Retrieved recipe is not a transmute recipe" | |
Description
This PR aims to add support for recipes including:
Target Minecraft Versions: any
Requirements: none
Related Issues: #5261