I have a few suggestions for the shaped builder API:

The current API statically allows to build incomplete recipes. For example a simple recipe with .aisle("a", " a").where('a', DIAMOND_AXE) could be changed to .aisle("a", " b") while forgetting to define 'b'. This would still compile, but eventually fail at runtime. I don't really see any benefit in using the weird separation.
So my suggestion would be to change aisle(String...) to aisle(Ingredient[][]), so the example from above could look something like this:

Ingredient a = ItemTypes.DIAMOND_AXE;
Ingredient s = Ingredient.builder().with(ItemTypes.LOG, ItemTypes.LOG2).build();
Sponge.getRegistry().getCraftingRecipeRegistry().register(plugin, "chopchop",
        CraftingRecipe.shapedBuilder()
                .aisle({{a, a}, {a, s}, {NONE, s}})
                .result(ItemStack.of(ItemTypes.DIAMOND_AXE, 1))
                .build());

aisle also doesn't really seem like the correct term here and with Java's array syntax it may not be the most usable experience, so I'd further suggest to replace aisle(Ingredient[][]) by something like row(Ingredient...). This method could then be called once for each row top to bottom. Additional helpers like emptyRow() (leave the row empty) and row(int, Ingredient...) (indent the row by n cells).

The above example:

Ingredient a = ItemTypes.DIAMOND_AXE;
Ingredient s = Ingredient.builder().with(ItemTypes.LOG, ItemTypes.LOG2).build();
Sponge.getRegistry().getCraftingRecipeRegistry().register(plugin, "chopchop",
        CraftingRecipe.shapedBuilder()
                .row(a, a)
                .row(a, s)
                .row(1, s)
                .result(ItemStack.of(ItemTypes.DIAMOND_AXE, 1))
                .build());

If the original intention of the string-pattern was to be able to reuse patterns, I can only say: functions.

@pschichtel It should be expected that people who want to write plugins/mods should test their plugin/mod before releasing it for use. If they test, they'll see an exception mentioning an ingredient was forgotten, just like they would if they forgot to specify something on another builder (the name on a boss bar builder, for example).
Additionally, vanilla Minecraft, as well as Bukkit and Forge, use the same aisle/where method (vanilla calls it pattern/key), so it's far from new:

• https://gist.github.com/kashike/eec72d61ce4dec229faf08b54db0e9c5
• https://github.com/Bukkit/Bukkit/blob/master/src/main/java/org/bukkit/inventory/ShapedRecipe.java#L34-L79
• https://github.com/MinecraftForge/MinecraftForge/blob/1.12.x/src/main/java/net/minecraftforge/common/crafting/CraftingHelper.java#L436-L493

@kashike by this logic, we should all start writing our projects in JavaScript or LUA. If we don't use the type-system, we even have one? Sure, if I write good code and have a 100% test coverage before every release, everything will work perfectly fine with either API. Then again, we're talking about Minecraft plugins, often written by beginner or intermediate skilled developers, barely tested if at all. Even skilled developers benefit from faster development cycles as the compiler can show the error much faster than deploying the plugin in a server, joining the server and testing the functionality.
Generally I think a builder that allows me to end the building process (e.g. call .build()) in an incomplete or inconsistent state is just bad. If other Sponge builders are like that, then that doesn't make the design good, just consistent(-ly sub-optimal).
Conceptually my suggestion isn't much different than the mentioned APIs, it just reduces the noise. Again: Just because others are doing it this way, doesn't imply it's a good way. I don't see any benefit in the shape definition using strings, let alone one that justifies sacrificing static verification support. And if the character based ingredient mapping is required for Minecraft/Vanilla compatibility, then that could easily be generated inside the builder implementation (assign a consecutive letter to every unique ingredient). This is nothing more than an implementation detail.
After all, isn't Sponge about improving upon its predecessors?

An exception would be thrown when build is called, as with a ton (all?) of our other builders when required information has not been provided. If someone can't be bothered to even start a server with their plugin, well, then that's just silly of them.

@kashike consider larger plugins, developed by fairly inexperienced developers (there exist quite a few of those, some of them widely used). Even my first Bukkit plugin was downloaded like 20k times on bukkit dev. I didn't always test every little feature. Who has even the time for that? Or the userbase to run beta phases?
Recipes are not bound to be registered on plugin startup (or are they?), so plugins could activate recipes upon certain events under certain conditions. Those would all need to be tested before a release.

A good builder kind of guides a user to a properly build object. Take a look at the jOOQ project, which is basically just a builder for SQL statements.

@pschichtel This "oh crap" feature presents on 90% of the builders. Then, in your theory, builders should be never used because they are extremely likely for noobs to have issues on.

A builder having an incomplete state at some point before build() is called is perfectly fine (normal even) as you almost always have certain bits of information that are required to be set. You can't have a recipe that has no result yet your not going to get a compile error if you don't pass that result to your builder. This is a fact of life with a builder pattern and is offset against the positives of the pattern.

Sure, an issue like that (registering a recipe later down the line, after a server has been running) can pop up. As can an NPE, SOE, IOOBE, etc.

@pschichtel This is harder than you've expected to screw up. I've seen many "stupid modders" doing silly things around, but never one who fucked up the most basic recipes. It is easy to use and easy to understand.

@liach Probably. That makes them consistent (which is not a bad property!), but in cases where a static verification is possible and easy, why not do it? I'm also perfectly aware, that these recipe builders are not hard to use. It's not about hard or easy, it's about preventing accidental mistakes, that can happen to anyone at any skill level in projects of any size.

@Deamon5550 It is fine sure. There are also cases where a complete static verification is not possible. For example if Vanilla MC doesn't support non-square, that could not be modelled in the builder, without potentially breaking support for mods. I also don't think every builder in Sponge MUST be completely statically safe, as that would introduce a lot of changes, but it would easily possible to model them that way. As mentioned, take a look at jOOQ.

@kashike Sure, any error can popup during run time, with or without recipes. This is not about making Plugins uncrashable, but about preventing one kind of error while at the same time increasing code quality.

While the static safety is a good reason to consider my approach, it is also not the only one (actually not even my main one, I might have focused to much on that). Has anyone a compelling reason to require these characters for the ingredient mapping? I don't see any reason why they need to be there. They don't add any new information to the code, just a potential (possibly unlikely) source of errors. The fact, that they can easily be generated confirms this. "Because it was always this way" should not be a reason in a discussion about a new API.

@Faithcaio Can you add a shortcut for plugin makers to turn an oredict entry into an ingredient?

@Faithcaio How are recipes with Logic handled? e.g. buckets returning empty buckets, clone recipes, Fireworks, etc.

@ryantheleach They are registered separately in code, not as jsons.

Vanilla has a concept of items that are a container for the bucket logic.
Anything else has to implement CraftingRecipe#getResult(grid) and CraftingRecipe#getRemainingItems(grid)

@Faithcaio As of Minecraft 1.12, recipes are organized into registries. I wonder if it is necessary to put recipes into catalog types and Use a class to contain all vanilla recipes like Keys class.

@Faithcaio Although people cannot remove recipe in the way they remove advancements, however, the recipes will be disabled by recipe books. Do we add recipe books as a way to disable, rather than ban, recipes?

We could add them, but i'd like to keep it separate from this PR.

So this and Impl in Common is basicly done.
SpongeForge will have to wait a bit.

Can we get this merged this weekend?

Any more concerns? If not I'll merge it today.

cd19310