Refactor recipes by kashike · Pull Request #1098 · SpongePowered/SpongeAPI

kashike · 2016-02-24T12:27:02Z

API | Common
Adds #1027
Resolves #1028

This PR changes ShapedCraftingRecipe (previously ShapedRecipe) from using an x and y int-based system to an aisle-based system, with char being used to get/set ingredients.

Example usage:

/*
 * 1 x 1
 *
 * ---------
 * | REEDS |
 * ---------
 */
// Re-create the sugar recipe
ShapedCraftingRecipe.builder()
    .aisle(
        "#"
    )
    .where('#', ItemStack.builder().itemType(ItemTypes.REEDS).build())
    .result(ItemStack.builder().itemType(ItemTypes.SUGAR).build())
    .build();

/*
 * 2 x 1
 *
 * ---------------
 * |    STICK    |
 * ---------------
 * | COBBLESTONE |
 * ---------------
 */
// Re-create the lever recipe
ShapedCraftingRecipe.builder()
    .aisle(
        "X",
        "#"
    )
    .where('X', ItemStack.builder().itemType(ItemTypes.STICK).build())
    .where('#', ItemStack.builder().itemType(ItemTypes.COBBLESTONE).build())
    .result(ItemStack.builder().itemType(ItemTypes.LEVER).build())
    .build();

/*
 * 3 x 3
 *
 * -------------------------
 * | STICK |       | STICK |
 * -------------------------
 * | STICK | STICK | STICK |
 * -------------------------
 * | STICK |       | STICK |
 * -------------------------
 */
// Re-create the ladder recipe
ShapedCraftingRecipe.builder()
    .aisle(
        "# #",
        "###",
        "# #"
    )
    .where('#', ItemStack.builder().itemType(ItemTypes.STICK).build())
    .result(ItemStack.builder().itemType(ItemTypes.LADDER).quantity(3).build())
    .build();

/*
 * 3 x 3
 *
 * ---------------------
 * | BED |  BED  | BED |
 * ---------------------
 * | BED | STONE | BED |
 * ---------------------
 * | BED |  BED  | BED |
 * ---------------------
 */
ShapedCraftingRecipe.builder()
    .aisle(
        "BBB",
        "BRB",
        "BBB"
    )
    .where('B', ItemStack.builder().itemType(ItemTypes.BED).build())
    .where('R', ItemStack.builder().itemType(ItemTypes.STONE).build())
    .result(ItemStack.builder().itemType(ItemTypes.BEDROCK).build())
    .build();

JBYoshi · 2016-02-24T12:31:51Z

Bed + stone (rock) = bedrock

😃

kashike · 2016-02-24T12:51:50Z

@JBYoshi You caught me. 😆

Mumfrey · 2016-02-24T13:41:28Z

This is a really nice idea.

Mumfrey · 2016-02-24T13:41:53Z

src/main/java/org/spongepowered/api/item/recipe/Recipe.java

+     * @return The resultant item type
     */
-    List<ItemType> getResultTypes();
+    ItemType getResultType();


Why remove the list?

As far as I can tell, recipes may only return a single ItemStack as a result, so a single ItemType makes sense - correct me if wrong.

Recipes can have more than one output, this was established a long time ago
#242 (comment)

@simon816 Okay, #1098 (comment) then.

Mumfrey · 2016-02-24T13:47:12Z

Just out of curiosity, where did the term 'aisle' come from?

ST-DDT · 2016-02-24T13:47:51Z

How would you handle data stuff?
Requires blue or red color and returns blue or red wool?

kashike · 2016-02-24T14:31:06Z

@Mumfrey I wrote this and stuck it in a branch around the same time I made #1028 - I think I stole the name from FactoryBlockPattern, though

@ST-DDT That's part of the ItemStack.

ST-DDT · 2016-02-24T14:40:49Z

@kashike So I have to create 16 recipies to create 1 item with 16 different data states?
What happens if I have two colors? 16x16 color => 256 recipies?

simon816 · 2016-02-24T14:42:45Z

These two API issues have been around for a long time, maybe you could see if you're able to fix them too? #463 #599 (just a thought)

kinggoesgaming · 2016-02-24T14:42:58Z

@ST-DDT I was hyped about this till you mentioned this

kashike · 2016-02-24T14:43:23Z

@ST-DDT That is what vanilla itself does, yes.

kashike · 2016-02-24T14:46:08Z

Since mods can create their own class off of IRecipe, I don't see a way to ensuring all recipes go through this - @Mumfrey?

simon816 · 2016-02-24T14:50:09Z

@kashike This is just for ShapedRecipes, there are other types of recipe too. That's what the more generic Recipe class represents (IRecipe). Also note this issue: #599

simon816 · 2016-02-24T14:51:50Z

src/main/java/org/spongepowered/api/item/recipe/ShapelessRecipe.java

         * @return fluent interface
         */
-        Builder addResult(ItemStack result);
+        Builder result(ItemStack result);


Should be varargs

Mumfrey · 2016-02-24T15:13:42Z

Since mods can create their own class off of IRecipe, I don't see a way to ensuring all recipes go through this - @Mumfrey?

This is why the work I'm currently doing on Mixin is so important. Yes it will allow us to address stuff like this.

simon816 · 2016-02-24T15:17:32Z

src/main/java/org/spongepowered/api/item/recipe/ShapedRecipe.java

-     * @return The ingredient
-     */
-    Optional<ItemStack> getIngredient(Vector2i pos);
+    Optional<ItemStack> getIngredient(char symbol);


I'm not sure on the usefulness of this method because it requires knowledge of how the recipe was created in the builder. I think an (x, y) coord would be better suited here.

Hmm yeah might as well I guess

simon816 · 2016-02-24T15:21:29Z

@Mumfrey in a mock-up implementation I did a long time ago (simon816/SpongeCommon@0184019) I was able to get around the problem by capturing the IRecipe instance when it gets registered and then attach a wrapping Sponge Recipe instance over it. (This was way before we used java 8)

stephan-gh · 2016-02-24T16:49:09Z

src/main/java/org/spongepowered/api/item/recipe/ShapedRecipe.java

+     * Gets the aisle.
+     *
+     * @return The aisle
+     */


Is that a copy? The Javadocs should mention this.

Zidane · 2016-02-24T17:09:21Z

@simon816

Yeah we aren't doing wrapping. This is solvable with changes coming to Mixin where we can mix into implementors of IRecipe.

felixoi · 2016-02-24T21:42:22Z

What's about an option to only assign an ItemType to a char? That would fix @ST-DDT 's problem

kashike · 2016-02-24T22:00:29Z

@Feeliiix Would you like something like...

Builder where(char symbol, @Nullable ItemStack ingredient) throws IllegalArgumentException;

Builder where(char symbol, @Nullable ItemType ingredient) throws IllegalArgumentException;

?

felixoi · 2016-02-24T22:01:33Z

Yes, exactly.

gabizou · 2016-02-24T22:02:54Z

It's very well possible that the where can take an ItemStackSnapshot, granted it'd be transitive to mod using ItemStack instances since there's damage values to consider, but for our sakes, it could very well match more than based on the damage value, but also the tag compound and any other custom data from plugins required.

kashike · 2016-02-24T22:03:52Z

We could accept ItemStack, ItemStackSnapshot, and ItemType?

Zidane · 2016-02-24T22:18:38Z

+1 to ItemStackSnapshot.

Zidane · 2016-10-02T06:02:57Z

src/main/java/org/spongepowered/api/item/recipe/furnace/FurnaceRecipe.java

+
+public interface FurnaceRecipe extends Recipe {
+
+    float getExperience(GridInventory grid);


Why GridInventory?

...and not something more targeted to an actual Furnace?

And it should return a doube. Missing javadoc.

me4502 · 2016-10-02T07:03:10Z

As a suggestion, should we have a 'CustomRecipe' or whatever, that accepts a predicate or function?

Basically that would allow plugins to create recipes similar to the fancy ones like armour dying.

liach · 2016-11-27T22:07:08Z

src/main/java/org/spongepowered/api/item/recipe/Recipe.java

     *
-     * @param grid An ItemGrid as input
+     * @param grid A grid inventory as input
     * @return A list of ItemStacks or {@link Optional#empty()} if the given


or {@link Optional#empty()} to or an empty list

liach · 2016-11-27T22:07:49Z

src/main/java/org/spongepowered/api/item/recipe/crafting/CraftingRecipeRegistry.java

+    boolean remove(CraftingRecipe recipe);
+
+    /**
+     * Gets a {@link Collection} of all recipes this registry is aware of.


Is this immutable?

liach · 2016-11-27T22:09:27Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapedCraftingRecipe.java

+         * Sets the aisle pattern for the shaped recipe.
+         *
+         * @param aisle A string array of ingredients
+         * @return fluent interface


liach · 2016-11-27T22:09:36Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapedCraftingRecipe.java

+     */
+    int getHeight();
+
+    interface Builder extends ResettableBuilder<ShapedCraftingRecipe, Builder> {


liach · 2016-11-27T22:11:48Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapedCraftingRecipe.java

+         * @param result The resultant stacks
+         * @return fluent interface
+         */
+        Builder results(Collection<ItemStack> result);


Why not accept Iterable?

liach · 2016-11-27T22:13:07Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapedCraftingRecipe.java

+/**
+ * A ShapedCraftingRecipe is a Recipe that has shape and fits into a grid.
+ */
+public interface ShapedCraftingRecipe extends CraftingRecipe {


Return a guava table of item stacks?

liach · 2016-12-06T22:01:09Z

@kashike Status?

Limeth · 2016-12-30T17:12:52Z

Can I offer my help here? I'd like to get recipes merged.

Limeth · 2016-12-30T17:14:28Z

I like the idea of using Predicates to check the item stacks.

kashike · 2016-12-30T17:27:08Z

@Limeth I've gone ahead and added you to my forks. Feel free to make new commits, just don't modify the existing ones. 👍

ST-DDT · 2016-12-30T21:01:16Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapelessCraftingRecipe.java

+         * @return The builder
         */
-        Builder addResult(ItemStack result);
+        Builder results(ItemStack... results);


This can be default implemented.

ST-DDT · 2016-12-30T21:01:29Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapelessCraftingRecipe.java

+         * @param ingredients The ingredients
+         * @return The builder
+         */
+        Builder ingredients(ItemStack... ingredients);


This can be default implemented.

ST-DDT · 2016-12-30T21:01:46Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapedCraftingRecipe.java

+         * @param result The resultant stacks
+         * @return The builder
+         */
+        Builder results(ItemStack... result);


This can be default implemented.

ST-DDT · 2016-12-30T21:03:12Z

src/main/java/org/spongepowered/api/item/recipe/furnace/FurnaceRecipe.java

+
+public interface FurnaceRecipe extends Recipe {
+
+    float getExperience(GridInventory grid);


And it should return a doube. Missing javadoc.

ST-DDT · 2016-12-30T21:05:18Z

src/main/java/org/spongepowered/api/item/recipe/crafting/ShapelessCraftingRecipe.java

+         * @return The builder
         */
-        ShapelessRecipe build();        
+        Builder results(Collection<ItemStack> result);


Why do you use a Collection here and for the ShapedRecipe an Iterable?

gabizou · 2017-01-10T19:51:48Z

@kashike I'm getting confused as to why there are now three PR's attempting to refactor recipes.

From what I can see, there's #1098 (this one), #1449 (Limeth's continuation of this one), and #1419 (Maxqia's refactor).

Normally, we should only have at most two PR's, but as it stands, there's some issues with all of them (this one being not updated). I'd like to get these PR's organized and some cleanup of reviews/comments from @SpongePowered/developers.

Maxqia · 2017-01-10T23:20:18Z

:P mine just kinda enveloped this one in response to Limeth's PR, My PR contains a lot of this PR's code...

kashike · 2017-01-11T00:03:39Z

@gabizou I gave this PR to @Limeth since they wanted to work on it and get recipes done. I didn't realise until now that a new PR was opened. I suggest @Maxqia and @Limeth work together to turn their two PRs into one.

Mumfrey reviewed Feb 24, 2016
View reviewed changes

simon816 reviewed Feb 24, 2016
View reviewed changes

kashike mentioned this pull request Feb 24, 2016

Begin work on recipes SpongePowered/Sponge#532

Closed

simon816 reviewed Feb 24, 2016
View reviewed changes

stephan-gh reviewed Feb 24, 2016
View reviewed changes

gabizou added the status: input wanted label Feb 25, 2016

gabizou changed the title ~~[WIP] Refactor recipes~~ Refactor recipes Aug 3, 2016

gabizou added the status: wip label Aug 3, 2016

Zidane reviewed Oct 2, 2016

View reviewed changes

Deamon5550 modified the milestones: Revision 6.0, Revision 5.0 Oct 18, 2016

liach suggested changes Nov 27, 2016

View reviewed changes

kashike added 7 commits December 8, 2016 02:44

Use an aisle instead of x,y with ShapedRecipe

7cd1d6e

some changes

58bb729

A few minor changes

39e75a1

CraftingRecipe

d27c112

And everything is going to the beat

6edee5d

I get carried away

327aba7

Like stars burning holes right through the dark

3e5e905

ST-DDT suggested changes Dec 30, 2016

View reviewed changes

Limeth added 2 commits January 1, 2017 20:54

added default implementations to several functions

e2822b2

don't use floats

4bd502e

This was referenced Jan 2, 2017

Generalize recipes and implement them #1449

Closed

Generalize recipes and implement them SpongePowered/Sponge#1119

Closed

kashike closed this Jan 11, 2017

Limeth mentioned this pull request Jan 14, 2017

Generalize recipes and implement them SpongePowered/SpongeForge#1188

Closed

This was referenced Jun 15, 2017

Recipe API #1582

Closed

Implement Recipe API SpongePowered/Sponge#1395

Closed


		public interface FurnaceRecipe extends Recipe {

		float getExperience(GridInventory grid);

Uh oh!

Conversation

kashike commented Feb 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JBYoshi commented Feb 24, 2016

Uh oh!

kashike commented Feb 24, 2016

Uh oh!

Mumfrey commented Feb 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mumfrey commented Feb 24, 2016

Uh oh!

ST-DDT commented Feb 24, 2016

Uh oh!

kashike commented Feb 24, 2016

Uh oh!

ST-DDT commented Feb 24, 2016

Uh oh!

simon816 commented Feb 24, 2016

Uh oh!

kinggoesgaming commented Feb 24, 2016

Uh oh!

kashike commented Feb 24, 2016

Uh oh!

kashike commented Feb 24, 2016

Uh oh!

simon816 commented Feb 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mumfrey commented Feb 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simon816 commented Feb 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zidane commented Feb 24, 2016

Uh oh!

felixoi commented Feb 24, 2016

Uh oh!

kashike commented Feb 24, 2016

Uh oh!

felixoi commented Feb 24, 2016

Uh oh!

gabizou commented Feb 24, 2016

Uh oh!

kashike commented Feb 24, 2016

Uh oh!

Zidane commented Feb 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

me4502 commented Oct 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kashike commented Feb 24, 2016 •

edited

Loading