EngineHub / CommandHelper

Rapid scripting and command aliases for Minecraft owners
https://methodscript.com
Other
119 stars 71 forks source link

add item_pre_anvil event #1384

Closed Lildirt closed 4 months ago

Lildirt commented 4 months ago

Kind of flying blind with creating a new event. It's been many years since I've written one, in an extension. :joy: Let me know if there's any validation you'd like for me to do on my end.

Example payload (pretty printed):

event_type: item_pre_anvil
first_item: {meta: {damage: 0, display: null, enchants: {silk_touch: {elevel: 1}, unbreaking: {elevel: 3}}, flags: {}, lore: null, model: null, modifiers: null, repair: 0, tags: null, unbreakable: false}, name: DIAMOND_PICKAXE, qty: 1}
item_repair_cost: 10
level_repair_cost: 10
macrotype: inventory
max_repair_cost: 40
result: {meta: {damage: 0, display: null, enchants: {efficiency: {elevel: 4}, silk_touch: {elevel: 1}, unbreaking: {elevel: 3}}, flags: {}, lore: null, model: null, modifiers: null, repair: 1, tags: null, unbreakable: false}, name: DIAMOND_PICKAXE, qty: 1}
second_item: {meta: {damage: 0, display: null, enchants: {efficiency: {elevel: 4}, unbreaking: {elevel: 3}}, flags: {}, lore: null, model: null, modifiers: null, repair: 0, tags: null, unbreakable: false}, name: DIAMOND_PICKAXE, qty: 1}
viewers: {Lildirt}
PseudoKnight commented 4 months ago

Looks like you matched 'viewers' from item_pre_craft, whereas we should probably get a player for both of these from the InventoryView. I don't think there can be multiple viewers, at least for these events on these GUIs. I would recommend removing 'viewers' and add 'player' for this event.

There's one hitch. InventoryView was changed from an abstract class to an interface a couple of weeks ago. Directly using methods from that interface may not work on older versions of the server. Might need reflection. Something like:

    @Override
    public MCHumanEntity getPlayer() {
        return new BukkitMCHumanEntity(ReflectionUtils.invokeMethod(e.getView(), "getPlayer"));
    }
PseudoKnight commented 4 months ago

Let me test the other event to see if reflection is needed here too. [edit] Confirmed. Needs reflection.

PseudoKnight commented 4 months ago

Actually can probably just do e.getViewers.get(0) instead of reflection. That's probably the way to go.

Lildirt commented 4 months ago

Actually can probably just do e.getViewers.get(0) instead of reflection. That's probably the way to go.

Pushed a commit. Something like that?

PseudoKnight commented 4 months ago

That looks fine. It looks impossible to have more than one viewer for an anvil. Unlike shared inventory views, the list of tracked viewers of an anvil is created every time a call to open an anvil is used.

PseudoKnight commented 4 months ago

Just noticed you have question marks in the description of the repair costs. Perhaps they were temporary?