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

Add recipe support #5204

Closed
wants to merge 9 commits into from
Closed

Add recipe support #5204

wants to merge 9 commits into from

Conversation

Pikachu920
Copy link
Member

@Pikachu920 Pikachu920 commented Nov 13, 2022

Description

Adds recipe creation section like this:

on script load:
  create a crafting recipe:
    key: "dirty-diamond"
    result: diamond named "Dirty Diamond"
    ingredients:
      dirt, dirt and dirt
      dirt, diamond and dirt
      dirt, dirt and dirt
	  
  create a furnace recipe:
    key: "dirty-diamond-boots"
    result: diamond boots named "Dirty Diamond Boots"
    ingredient: diamond
    cook time: 1 second
    xp: 500
	
  create a campfire recipe:
    key: "chainmail-boots"
    result: chainmail boots
    ingredient: diamond boots
    cook time: 1 second
    xp: 500

Target Minecraft Versions: Any
Requirements: N/A
Related Issues: probably but I'll find them later

@AyhamAl-Ali AyhamAl-Ali added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Nov 13, 2022
@Ankoki
Copy link
Contributor

Ankoki commented Nov 15, 2022

Just a suggestion but do you think it would be worth this being a structure when they're released? I feel like as these aren't dependent on any event these can be utilised.

on join:
    send "§eWelcome §7%player%§e!"

create crafting recipe for gold sword with key "test":
    dirt, dirt, air
    air, dirt, air
    air, air, air

I just feel this is alot easier tbh, not that big of a difference but a nice enhancement to have

@Fusezion
Copy link
Contributor

Just a suggestion but do you think it would be worth this being a structure when they're released? I feel like as these aren't dependent on any event these can be utilised.

https://discord.com/channels/135877399391764480/836220422223036467/1041449280776458300

I believe there was talk about whether they should be made as a structure but from what I remember we mostly put it down
not fully sure what the reasons were

@Ankoki
Copy link
Contributor

Ankoki commented Nov 15, 2022

I believe there was talk about whether they should be made as a structure but from what I remember we mostly put it down not fully sure what the reasons were

I see the reasoning here tbh, however arent structures able to be used as sections? i could be missing the ballpark completely but supporting both seems like something that structure is in range for, and if not that would be a nice imrpovment to structures

@Fusezion
Copy link
Contributor

I believe there was talk about whether they should be made as a structure but from what I remember we mostly put it down not fully sure what the reasons were

I see the reasoning here tbh, however arent structures able to be used as sections? i could be missing the ballpark completely but supporting both seems like something that structure is in range for, and if not that would be a nice imrpovment to structures

I'm aware that you can add the entry stuff as it's public but structures are top level events so they should be limited to the same area events, functions and commands are right?

@TheLimeGlass TheLimeGlass mentioned this pull request Dec 23, 2022
1 task
@TheLimeGlass TheLimeGlass added the 2.8 Targeting a 2.8.X version release label Jan 3, 2023
@Pikachu920 Pikachu920 closed this Mar 21, 2023
@Fusezion
Copy link
Contributor

Noo! we want theses

@Pikachu920 Pikachu920 reopened this Mar 21, 2023
Copy link
Collaborator

@TheLimeGlass TheLimeGlass left a comment

Choose a reason for hiding this comment

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

There should also be a Structure for recipe registration. This will allow for parsing time registration. Makes it easier and simpler for users while also having the sections.

Tests can be added.

@sovdeeth
Copy link
Member

sovdeeth commented Mar 25, 2023

State of the syntax at time of writing:

create a crafting recipe for diamond named "Dirty Diamond" with the key "dirty-diamond":
    shape:
        dirt, dirt and dirt
        dirt, diamond and dirt
        dirt, dirt and dirt

Personal thoughts on the existing syntax:

  • I like that's it's a section. Most use cases of custom craft involve specific items that are rather unwieldy as literals (or impossible to write as literals), so the ability to use expressions and variables in the ingredients is, in my eyes, a key and essential feature.
  • I like that it's easy to see the recipe at a glance. Laying it out in a 3x3 makes it much easier to read and see what the shape looks like, versus a 1x9 list.
  • I'm not a huge fan of shape:. Since it's the only field in the section, I think it's rather superfluous. I get the idea that it's kind of a lead-in to writing the shape, and kinda takes the place of with the shape "XYZ", but since it's the only thing you're going to put there anyway it feels more like just an unnecessary word to type than a helpful label. If there was a result field too, it'd make more sense in my opinion.
    • After slight discussions w/ pickle and pikachu, I also like the idea of keeping shape: but also moving result: into a field instead of in the parent node.
create a crafting recipe with the key "dirty-diamond":
    result: diamond named "Dirty Diamond"
    shape:
        dirt, dirt and dirt
        dirt, diamond and dirt
        dirt, dirt and dirt
  • I'm ok with the lack of an effect to start the shape lines. It's a new thing for skript, but I don't think it's bad. The section kind of acts like the effect for all the shape lines, making this more like a multi-line effect. I think it's intuitive and makes sense. However, I do have some alternatives if others find this too much of an upheaval.

Personal thoughts on additions:

  • I agree that a structure version could be useful. However, if it doesn't allow expressions in its ingredients, as I understood is the case in a discord discussion, it'd be rather limited to just very simple recipes of generic material to generic material. This doesn't mean it's not worth adding, it's just that we should make sure it's well documented to ensure that the section's abilities and the structure's abilities are not confused.
  • For the shape lines, my alternative suggestion:
crafting recipe:
    set [ingredients of] row 1 to x, y, z
    set [ingredients of] row 2 to x, y, z
    set [ingredients of] row 3 to x, y, z

This is mainly just to change the section as little as possible, but allow it to maintain the property of having an effect/section on each line of code. It could be extended with:

crafting recipe:
    set ingredients to x, y, z, a, b, c, 1, 2, 3

if desired.

To summarize:

I like the current implementation of the syntax, minus the shape: bit, which I find a bit unnecessary (edit: I think including both result: and shape: is the best option). I think the lack of effect on the shape lines is perfectly fine, though I have an alternative if needed. In my experience, specific custom items are very commonly used in custom recipes, which means that if a structure was made, it'd be of somewhat limited use and would have to be well documented as to the differences with the section version. I don't think that's a reason not for it to exist, though.
All in all, I think this is a valuable addition to skript, and I like the current direction.

@TheLimeGlass
Copy link
Collaborator

Potentially add support for KnowledgeBookMeta? #617

@TheLimeGlass
Copy link
Collaborator

Issue related #5261

@Pikachu920 Pikachu920 changed the title Add SecRecipe Add recipe support Mar 25, 2023
@Pikachu920 Pikachu920 marked this pull request as ready for review May 3, 2023 02:57
static {
Skript.registerEffect(EffDiscoverRecipe.class,
"make %players% (discover|unlock) recipe[s] %strings%",
"make %players% (undiscover|lock) recipe[s] %strings%",
Copy link
Member Author

Choose a reason for hiding this comment

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

whitespace is messed up

@SuppressWarnings("unchecked")
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) {
discover = matchedPattern % 2 == 0;
players = (Expression<Player>) exprs[matchedPattern <= 1 ? 0 : 1];
Copy link
Member Author

Choose a reason for hiding this comment

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

store the condition in a variable

}

try {
Bukkit.getServer().addRecipe(recipe);
Copy link
Member Author

Choose a reason for hiding this comment

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

check this? it seems to return a boolean not throw an exception now that i look again

@Moderocky Moderocky force-pushed the master branch 2 times, most recently from bd134d0 to 3f08853 Compare September 16, 2023 16:59
@Moderocky Moderocky changed the base branch from master to dev/feature September 18, 2023 09:45
@sovdeeth sovdeeth removed the 2.8 Targeting a 2.8.X version release label Dec 30, 2023
@sovdeeth
Copy link
Member

Closing due to inactivity.

@sovdeeth sovdeeth closed this Jul 16, 2024
@APickledWalrus APickledWalrus deleted the feature/crafting branch October 13, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants