[1.8] How to "hook" into crafting table for custom item treatment

[ZTagre]

Hey folks!

 

   I am currently working on a "concept" item, and I need just a little help with one aspect of it.

 

Here's what I'm doing in a "nutshell": I've created a "magic staff", which has custom, "insertable" charge-orbs;

each orb gives the staff a single "magical use", before the ORB ONLY is destroyed. You craft the staff from three pieces, and then each orb has its own recipe as well. Now, for example, one of the ORB CHARGES makes the staff a "teleportation staff" when it's INSERTED into it (added to it using the crafting table, pretty basic...). When the teleportation staff as such is [shift]-right-clicked, I store the location of the block that the staff was pointing to in the staff's NBT compound. This is the location later used to teleport the player. Pretty straight-forward, so far...

 

   Now, I want to be able to REMOVE the ORB from the staff (also in the crafting table) once it has been "charged", for storage, and later use (by once again re-inserting it into the staff...). BUT, I need to somehow "copy" the NBT data from the complete teleportation staff to the "charged" teleporation ORB because it will of course be a new, seperate item (got me?). Thus, the ORB, while seperated from the staff, still contains the selected location for the teleport. When the ORB is later re-attached (re-inserted) into the magic staff, the staff once again becomes a "teleporation" staff, and the NBT data saved within the ORB now once again gets copied to the teleporation staff's NBT compound.

 

   So, what I need to do (sorry if this was long-winded) is somehow "intercept" the crafting table "handler", and a) Check if the contents of the crafting grid is a teleporation staff, and then b) if it is, then c) reate the ORB, with the staffs NBT data "copied" to it as the crafting result, whilst leaving the "normal", empty staff in the crafting grid.

 

   As a secondary stage, I will also need to be able to re-attach the "charged" ORB to the normal staff, with the resulting teleporation staff having the NBT data from the charged orb.

 

   But, to do any of this, I must somehow "intercept" the crafting table code, and I have looked long and hard at all of the vanilla code which pertains to crafting in any way. I know that , in later versions of Minecraft (later than 1.8, that is) there is something called ICraftingHandler, which would probably make this a lot easier; sadly, I don't think this will actually be that simple (but I could be wrong!).

 

   ANY pointers on how to begin here will be much appreciated. With every passing day, with the help of various tutorials, reading the vanilla source, and with help from you folks, I am getting better at this. Soon, I hope to be able to help someone who is much like me today!

   Thanks in advance,

   ZTagre.

Edited February 13, 2017 by ZTagre
Solved

[Choonster]

You need to create your own implementations of IRecipe to handle adding orbs to and removing orbs from the staff. You can probably extend an existing IRecipe implementation like ShapedOreRecipe/ShapelessOreRecipe.

 

Register your recipe classes with RecipeSorter and add instances of them to the recipe list using GameRegistry#addRecipe(IRecipe).

 

Side note: There is no ICraftingHandler in any modern version of Forge. It looks like ICraftingHandler was a predecessor to events like PlayerEvent.ItemCraftedEvent and was removed in 1.7.2 (I never encountered it myself).

Edited February 12, 2017 by Choonster
Added missing word

[ZTagre]

So, all I really need to do is create an IRecipe implementation? That's it? Wow, I had that WAY overcomplicated! I'll need to have a think about it, but I'm pretty sure I've got it covered...thanks, bro!

 

   As per the info on ICraftingHandler - it seemed like a pretty cool thing; I saw a piece of sample code, and I thought it looked like just what I needed! I have included it as an attachment to this reply, if you'd like to see it!

 

Thanks a bunch!

   ZTagre

Minecraft forge tutorials sea2 - item taking damage when used in crafting.Source.txt

[ZTagre]

Thanks for your input, diesieben07! It sure looked good "from a distance", and when I could not find ICraftingHandler in the vanilla 1.8 code, I was quite disappointed! I assumed, once again, that it was something "new", added to later versions of MC / Forge, and that I would sadly have to do without! It's kinda nice to know that it wasn't "all that"!

 

   ZTagre.

[Choonster]

47 minutes ago, ZTagre said:

Thanks for your input, diesieben07! It sure looked good "from a distance", and when I could not find ICraftingHandler in the vanilla 1.8 code, I was quite disappointed! I assumed, once again, that it was something "new", added to later versions of MC / Forge, and that I would sadly have to do without! It's kinda nice to know that it wasn't "all that"!

 

   ZTagre.

 

For future reference: Any code that imports classes from the cpw.mods.fml.* packages was written for 1.7.10 or earlier. FML was moved to net.minecraftforge.fml.* in 1.8.

Edited February 12, 2017 by Choonster
Packages, not namespaces

[ZTagre]

1 minute ago, Choonster said:

 

For future reference: Any code that imports classes from the cpw.mods.fml.* namespaces was written for 1.7.10 or earlier. FML was moved to net.minecraftforge.fml.* in 1.8.

Good to know! Thanks, Choonster!

[ZTagre]

Just  FYI, people:

   So that you know your help has been useful, I would like to report that the item I was describing at the start of this topic is now FULLY FUNCTIONAL, and I have you folks to thank for this! So, in the spirit of sharing with the community, I have attached a link to the working Minecraft 1.8 source for my IRecipe implementation "RecipeDetachChargedOrb", which is fully commented, in case anyone may be interested! I have not attached the source for the reverse recipe "RecipeAttachChargedOrb", but if anyone is interested in THAT, I will happily post that as well!

 

   Thanks again!

   ZTagre.

 

PS - I have a strong chin so if anyone wishes to critique my work, go right ahead! I welcome all insights and suggestions! Also, before you begin : I LIKE (and probably OVERUSE) the "this." when referencing class member variables; I am aware I don't (always) need to do this :-}

RecipeDetachChargedOrb.java

[Choonster]

Your IRecipe shouldn't be storing any state between methods, each method should only do exactly what's required of it without any side-effects:

• matches should find the staff in the crafting grid, make sure it's valid and that there aren't any other items.
• getCraftingResult should find the staff in the crafting grid and return the output item with the appropriate NBT or return null if it didn't find the staff.
• getRemainingItems should find the staff in the crafting grid and set that slot of the output array to the uncharged staff.

 

You completely ignore the output stack passed to the constructor by overwriting it in matches. I suggest taking any items used by the recipe as constructor arguments, storing them in final fields and then using them in the appropriate methods.

 

There's no need to explicitly fill an array with null, null is the default value of all reference types.

 

There's no need to copy an ItemStack that's just been created and won't be stored anywhere.

[ZTagre]

Wow! Thanks, Choonster!

 

   Geez, I'm kinda surprised that it actually works, now! Let's see, what I did wrong:

   1) I actually used a new ItemStack to set the OUTPUT stack, thus OVERWRITING the one I passed in with the constructor! - yep, I see that quite clearly now!

 

 2) I got WAY carried away doing most of the stuff in matches(); I realize I was not sure about just WHAT to do in most of the other methods, so I hung everything on matches; when it worked, I assumed that was okay. My bad! BUT - in my own defence, let me just say that if you were to look at (say) RecipeFireworks for MC 1.8, they did a LOT of stuff in matches(), and so very little in any of the other methods! It may have been a bad example, but , hey, it was VANILLA code :-}

 

 3) Sorry, exactly WHERE did I "copy" an ItemStack that "wouldn't be stored anywhere" (I mean, if more than one, then WHICH) ? I used output_stack.copy() because the VANILLA code did, in getCraftingResult(...). Otherwise...(?)

 

   Thanks again for the input! Much appreciated...

     ZTagre.

[Choonster]

29 minutes ago, ZTagre said:

 2) I got WAY carried away doing most of the stuff in matches(); I realize I was not sure about just WHAT to do in most of the other methods, so I hung everything on matches; when it worked, I assumed that was okay. My bad! BUT - in my own defence, let me just say that if you were to look at (say) RecipeFireworks for MC 1.8, they did a LOT of stuff in matches(), and so very little in any of the other methods! It may have been a bad example, but , hey, it was VANILLA code :-}

 

I didn't realise RecipeFireworks did that. I suppose it works, but having implicit side effects tends to make code less robust and harder to follow.

 

 

29 minutes ago, ZTagre said:

 3) Sorry, exactly WHERE did I "copy" an ItemStack that "wouldn't be stored anywhere" (I mean, if more than one, then WHICH) ? I used output_stack.copy() because the VANILLA code did, in getCraftingResult(...). Otherwise...(?)

 

You copy the stack variable in getRemainingItems before putting it in the array, but the original value is never stored or used anywhere else.

[ZTagre]

 

Yeah, you're right! I didn't need that .copy() method there...; I think I put that in while trouble-shooting, and just forgot about it! You only have to set the array index to the value of the stack "directly"...

 

   So, instead of:

    itemstack_array[this.source_item_index] = stack.copy();

 

it should just be:

    itemstack_array[this.source_item_index] = stack;

 

   Got it! Thanks!

[ZTagre]

Hey, Choonster, if I could ask you just one more quick thing before I do the re-write on this?

 

   Is it NOT okay to "save" the index of the staff that was found in the crafting grid in matches() in a private class variable, for later use by the other methods? I just thought that by saving it, I would be saving a few more lines of iterative code, "looking" for it all over again...; if this is wrong, I don't want to repeat that mistake :-}

 

   As a class private member variable, I know no other class code can touch it, so I felt this would be "safe" as you say "across states"...; if this is not the case, then maybe explain?

 

   Thanks a bunch.

 

 

[Choonster]

39 minutes ago, ZTagre said:

   Is it NOT okay to "save" the index of the staff that was found in the crafting grid in matches() in a private class variable, for later use by the other methods? I just thought that by saving it, I would be saving a few more lines of iterative code, "looking" for it all over again...; if this is wrong, I don't want to repeat that mistake :-}

 

   As a class private member variable, I know no other class code can touch it, so I felt this would be "safe" as you say "across states"...; if this is not the case, then maybe explain?

 

It will probably work, but I wouldn't recommend it.

 

Each method should be self-contained and not rely on other methods being called before or after it. If the matches method saves the index, the other methods will only work if they're called after it.

Edited February 13, 2017 by Choonster

[ZTagre]

You know, I considered that EXACT POSSIBILITY when I saved the index; like "what if the other functions don't get the correct value for the staff index", and then I reasoned:

   a) If matches() has NOT yet been executed, then exactly why is/are the other method(s) even being executed at all? I mean, if the physical "test" for a legitimate recipe has NOT PASSED MUSTER, then no item should be available as an output from said recipe.

  b) What would method getCraftingResult() use as the result of a recipe that had not yet gone through the check(s) done by matches()? Would it be LOGICAL to show that "result" item? If so, would the "result" item appear only momentarily, until matches() failed?

 

   Now, please understand, I fully agree with you that each method should be "self-contained", as you say, and should not rely on any previous method having been called first. It's just that, when I considered what the "order of operation" should be within the entire item-crafting process, the only logical order I could SEE was that matches() would be called almost immediately, and at the VERY LEAST before getCraftingResult(), getRecipeOutput() or getRemainingItems(), because each of those methods should do NOTHING when the recipe is a non-match. And those methods are the only ones where I might have used this saved index!

 

   It seemed like a "no brainer", really :-}

   So that's why I felt it would be safe to save this index for later use...

 

   Once again, I thank you for your advice!

   ZTagre.

[Animefan8888]

I believe Choonster is saying this for inter-mod compatibility. Say what would happen if a mod did call them "out of order". Your I recipe will think that there is a staff at the last index specified by the matches method (or 0 because that is the default value).

[ZTagre]

14 minutes ago, Animefan8888 said:

I believe Choonster is saying this for inter-mod compatibility. Say what would happen if a mod did call them "out of order". Your I recipe will think that there is a staff at the last index specified by the matches method (or 0 because that is the default value).

So, you mean like if I was perhaps using some other mod along with my own - a mod that (for whatever reason), needed to "modify" the order in which the IRecipe stuff was done, then it is possible that matches() might then NOT be the primary function, called before the other methods as one would expect? Wow; I don't know...

 

   But, I do see your point, and, like I said to Choonster, I AGREE that keeping each method and the data it works with "encapsulated" as a unit is a good design consideration. I really was simply trying to save some "clock cycles" (so to speak) .

 

   I'm an old C and assembler coder from way way back, and much of the things I do in code is because, unlike with Java, NO guarantees were made about the default values of created variables, etc; if you did NOT explicitly SET a value at creation time, then very bad things could come along later and bite you on the you-know-what; also, saving every bloody bit of clocks was an issue as well back then! Heck, I would not even DREAM of performing a loop more than ONCE, if I felt that I could get away without doing so!!!

 

   So, yeah, I'm finding some of these aspects of Java hard to get used too! I will though, I promise!!!

 

   Thank you for your input, as well :-}

   Cheers,

   ZTagre.

[Draco18s]

I suppose if you want to save the index you could, but I'd do it like this:

 

if the index is a valid number (e.g. not -1 and within the array bounds), check the grid location at that spot.

If it's a staff, awesome, you found it, good job.

IF NOT, then iterate over the whole grid.

 

You save some CPU by caching an index, but have a fallback if the staff isn't there or the index is bad.

[ZTagre]

5 minutes ago, Draco18s said:

I suppose if you want to save the index you could, but I'd do it like this:

 

if the index is a valid number (e.g. not -1 and within the array bounds), check the grid location at that spot.

If it's a staff, awesome, you found it, good job.

IF NOT, then iterate over the whole grid.

 

You save some CPU by caching an index, but have a fallback if the staff isn't there or the index is bad.

Yes, that is of course VERY PRUDENT advice! I check for null all the time; its just a few short if-statements to do a better job!

   In retrospect, that is was I would have done, were it not for merely trying to get it to WORK in the first place, which I was really focused on! Checking for null alone simply doesn't cut it...

   Now, while I have your attention (so to speak); what are your feelings on doing what I did within the matches() method (other than, of course, being a complete DOLT and overwriting the saved output ItemStack with a new one, which I can't believe I actually DID :-{ ) ? (That, btw, was fixed IMMEDIATELY!)

   It sure LOOKED like it did the job, much the same way as what I loosely based it on (RecipeFireworks, MC v1.8). I could EASILY change it so that is copies the NBT data from the staff to the the orb within the getCraftingResult() method, but how "dangerous" or "hard to maintain" is it really, just the way it is?

   I'm only looking for other opinions...; I already know Choonster's thoughts on this!

 

Thanks bud!

   ZTagre

[Draco18s]

Looks fine to me, I guess.

I haven't messed with IRecipes much. I did one.

https://github.com/Draco18s/ReasonableRealism/blob/master/src/main/java/com/draco18s/hardlib/api/recipes/RecipeToolMold.java

Which was based on regular recipes (I forget what I used as a base).