Create version of PrepareItemCraftEvent for crafters

[Pitrex111]

Is your feature request related to a problem?

Some of my plugins crafting logic depends on PrepareItemCraftEvent and affecting the result. Because crafter does not invoke that event and has no special events they break my crafting plugins

Describe the solution you'd like.

Either include crafter in PrepareItemCraftEvent or create new event for crafter that inherits on the same base as PrepareItemCraftEvent. (My main concern is to not have double implementation for crafters)

Describe alternatives you've considered.

Probably interacting with crafter invokes inventory events, I haven't looked into this yet, but I could probably listen to some eventory click event check the crafter inventory and fabricate PrepareItemCraftEvent (crafter inventory i bare bone but I excpect that inside there is at least 10 elements array when 10th element is the result)
But this won't work perfectly as there are many inventory based entry points (player, hoppers, usage of crafter) that might change the state of result item and it would be great to have one common point when paper looks recipes for crafter

Other

No response

[Machine-Maker]

Yeah, this sounds like a good idea for me. We can't just directly reuse PrepareItemCraftEvent because that event requires having an InventoryView which only exists with a "player" actually viewing the inventory. But we could have a supertype of that event that is also then fired for crafters that has everything else.

[MrPowerGamerBR]

There also needs to be a CraftItemEvent for crafters too, which like PrepareItemCraftEvent it cannot be repurposed for crafters as-is because it requires a InventoryView

[lynxplay]

#11082 exists as a minimal base for a stable 1.21, mostly focused on protection plugins.
I'd love some feedback from the people involved here regarding that as a base or if their plugin really needs something more for 1.21 to not completely fail.

[Grabsky]

LGTM, explanation regarding "prepare" variant of this event makes a lot of sense, thanks for point this out.

[Pitrex111]

@lynxplay This event fires when Crafter is powered if I understood the code correctly. It would be highly beneficial to get also an event that fires when recipe used is changes (as when PrepareItemCraftEvent is invoked)

[lynxplay]

How so? The recipe is not evaluated until the item is about to be crafted. What benefit does PrepareItemCraftEvent for Crafters yield you here?

To explain this a bit further, the "result" you see when you open a crafter inventory is entirely local to said menu.
It does not exist in the crafter block entity whatsoever.

[Pitrex111]

Actually there are 2 cases where recipes are used. One where you open the crafter gui, you have a crafting grid and result showing of what crafter will produce.
So in case I have custom item using lapis lazuli item id and I don't want this item to be convertable into blue dye I listen to PrepareItemCraftEvent and set result to null/air. This way player don't get confused about this item. So for crafter also if a player will place that item into crafter I don't want him to see blue dye in the result window in a gui.
Second thing, I have a simple recipe declared and inside the event I change the lore and components of resulting item stack, for crafting I use PrepareItemCraftEvent to change the result so that player can see what they shall craft. I also would want to see those changes reflected in crafter gui.
Those 2 cases won't be solved with just an event that triggers when crafter is powered and item is being crafted. (That's also why there are 2 event types for player crafting PrepareItemCraftEvent to change the possible result of crafting and CraftItemEvent for the exact moment when crafting happens)

[lynxplay]

Okay yea, but those changes are purely visual then on your end.
As stated on the PR, there is a use for such an event, but it is going to need more work.

CrafterCraftEvent sounds like it solves all but your visual issues, which means we can merge this PR to progress to stable 1.21 and work on something for your plugin to modify the ephemeral result shown in the GUI later.

[Pitrex111]

If we're talking about completelly bare minimum then I guess yeah, itąs enough for me to not block crafters on update, but not enough for me to say this can be closed.

Btw sorry for offtop, but while I have your attention could you look into #10973
It solves an issue with crafting recipes and is already tested by me that it works but for 2 weeks nothing went forward. Could you check into that PR and see if it can also be delivered?

[lynxplay]

Yea no worries I wasn't suggesting closing this. Yea the goal is bare minimum, just so shit doesn't explode. We can deliver and properly close this discussion once we are in the normal stable version lifecycle.

[lynxplay]

2a39276 has been merged.

As described in the javadoc, sadly upstream decided to also propose this event.
We preemptively ported their proposed changes into an experimental event.
If everything goes well, when they merge said PR and nothing changed, we can just drop the experimental annotation.

However, as we do not have control over what upstream does, they may change some methods around, in which case, there will be a breaking change on paper. This preliminary event pull should enable everyone here to do the bare minimum (see discussion above, we still need to work on a way to also fix the visuals in the CraferMenu), but I'd heavily suggest following the dev-announcment discord channel to get notified if we have to pull breaking changes to this event down the line.