on right click of: does not support offhand, and cancelling events is broken

[TsCode1]

Description

There are 2 bugs. For one, "on right click of" does not include offhand. This can cause huge exploits on a lot of servers that are using this trigger and should be fixed.
the second bug is even worse because it completely breaks all cancel events and requires a server restart, just ruins the server completely
2, when cancelling 2 events at the same time that are on the same script, then skript breaks and cancels the event continuously without a trigger.

Steps to Reproduce

1. on rightclick holding a egg: cancel event will not cancel event when right clicking on offhand.

2. to reproduce second bug, have these 2 things for example: `on rightclick holding a egg:
   if player does not have permission "egg.use":
   send "no 1."
   cancel event

on rightclick holding a bow:
if player does not have permission "bow.use":
send "no 2"
cancel event`

try rightclicking them both at the same time (or put one item in your offhand and right click twice and it will break immedietly) and then it will stop checking the permission and will not send a message, but will rather just cancel the event forever and u cant use the items in any way, cant even break blocks with them.

Expected Behavior

I expected for right click trigger to include offhand ( there is no other offhand support) and I expected cancelling events to not leak and break the server completely by ruining the items

Errors / Screenshots

will do if necessary, the bugs are clear

Server Information

• Server version/platform:
• Skript version:
  `[16:03:54 INFO]: This server is running Paper version git-Paper-1593 (MC: 1.12.2) (Implementing API version 1.12.2-R0.1-SNAPSHOT)
  [16:03:54 INFO]: You are 2 version(s) behind

ver skript
[16:04:00 INFO]: [MyCmd] This command have ADD_PERMISSIONS type
[16:04:00 INFO]: Skript version 2.3.1
[16:04:00 INFO]: Customize Minecraft's mechanics with simple scripts written in plain English sentences.
[16:04:00 INFO]: Website: https://skriptlang.github.io/Skript
[16:04:00 INFO]: Authors: Njol, Mirreski, bensku, TheBentoBox, Snow-Pyon, Pikachu920, Nicofisi, tim740, Tuke-Nuke, TheLimeGlass, xXAndrew28Xx, Sashie, RoyCurtis, nfell2009, Syst3ms and Blueyescat`

Additional Context

[Itzdlg]

Skript needs to rewrite the EvtClick.java as right now its quite a bit messy and buggy.

[TsCode1]

on shoot: if projectile is an egg: if shooter does not have permission "egg.use": send "no" cancel event even this breaks. just put this in an empty script. then use eggs a lot, maybe in both hands, or just click random things, and the cancel event won't listen to the trigger anymore, will just cancel every click action while holding egg including breaking blocks while holding the egg, and right clicking too. I think cancel event is very broken

[Snow-Pyon]

@dad3 it isn't broken, you can blame Spigot on all those "bugs" when cancelling events, Skript doesn't add any logic when it comes to cancelling events.

Skript needs to rewrite the EvtClick.java as right now its quite a bit messy and buggy.

Not really, it is ok, not the best thing but it works as people expect it to work, or at least it did.

Can someone confirm these bugs?

[TrademarkTM]

Tested the on shoot code, it cancelled all the clicks with no problems at all. No eggs thrown.

on rightclick, however, has some pretty strange bugs:
Tested on 1.13.2 with Skript 2.3.1

on rightclick holding an egg:
	cancel event

• If you have eggs in your offhand slot (and any other item in your main hand), it does not cancel the event.
• If you're looking at an entity, it does not cancel the event.
• If you quickly change the block you're looking at, it sometimes does not cancel the event.
• You don't even need to use holding an egg. Cancelling the entire event also has that bug.

PlayerInteractEvent is pure garbage though. Not sure if it's Spigot's or Mojang's fault, but even I could make something better.
In 1.13.2, this is the behaviour with these events (tested with skript-mirror):

• cancelling a PlayerInteractEntityEvent does not prevent a player from shooting a projectile.
• PlayerInteractEvent gets called twice in some cases (checks for both hands, if it's a valid click it gets called).
• PlayerInteractEvent is not called when throwing eggs and not aiming at any block. However, if you aim at a block it gets called and can be cancelled.

My skript-mirror code (pretty straightforward):

import:
	org.bukkit.event.player.PlayerInteractEvent
	org.bukkit.event.player.PlayerInteractEntityEvent

on PlayerInteractEvent with priority highest:
	broadcast "&dINTERACT"
	# event.setCancelled(true)
	
on PlayerInteractEntityEvent with priority highest:
	broadcast "&bENTITY INTERACT"
	event.setCancelled(true)

[TsCode1]

I am not blaming spigot because all the other plugins managed to find a way to cancel event without it leaking somehow. I think this can be fixed in skript by trying a different method or changing the code. As for the items breaking, it is really easy to reproduce but just not clear exactly what causes it. If you cancel event of 2 items and then put 1 in your offhand, spam click both, then they will stop working properly and ignore the trigger. It will cancel event on these items for other irrelevant clicks. I think that's the easiest way to reproduce the second bug, all though I've done it without using offhand and sometimes doing nothing at all.

[Snow-Pyon]

I wasn't talking about the bug you mentioned in the issue, I was talking about your other comment.

And you can't compare plugins to Skript either way, plugins use the exact events with specific checks and workarounds if needed, in events like the click event, Skript makes 20 lines of code a simple one liner, the fact that this doesn't perform as well as it should is just the fact that Spigot doesn't have proper API implementation.

[bensku]

Skript merges two Bukkit click events to one for scripts. Since that event must be cancellable, it is not possible to wait until end of tick to see how many events were fired. And to make matters worse, passing the wrong event is not actuallt canceling what user wanted to do.

Skript tries to guess which click event is right one - i.e. properly cancels the action - using lookup tables. Said tables have been built using basically reverse engineered information. It is possible that they are not 100% accurate.

Event-item, which should be what click events are also filtered against, is the item that (according to lookup tables) was used. Now, I'm afraid that this might not make much sense with some syntaxes of click event - could be a bug.

Again, it could also be a bug in lookup tables. I'm not sure...

As a side note, entity click events are generally much more simple to deal with. Well, until you need armor stand support. Then it gets ugly.

[TheBentoBox]

I am not blaming spigot because all the other plugins managed to find a way to cancel event without it leaking somehow.

I also want to note that you're the only one experiencing this. What you're saying is that your event triggers eventually just "break" and no longer work. I've never heard that reported before and have never experienced it myself, you say it's easy to reproduce but the scenarios you provide aren't breaking anything for anyone else. Skript handles interact events fine, or at least fine enough to not cause the issues you're describing, If you can provide any actual code which, for other people, reproduces the issue, then it's something we can look into.

The reason the egg right-click event isn't always firing is due to what @TrademarkTM showed. The Skript click events operate off of player interaction events, which are not always fired by Spigot. If it doesn't fire the event in a certain scenario, then Skript won't fire a click event either. There's no magic plugin that is making Spigot fire click events there either -- if they want to cancel egg shooting, they listen for shoot events, not click (interact) events, as should you. It's not that hard.

[TsCode1]

I wasn't talking about the bug you mentioned in the issue, I was talking about your other comment.

And you can't compare plugins to Skript either way, plugins use the exact events with specific checks and workarounds if needed, in events like the click event, Skript makes 20 lines of code a simple one liner, the fact that this doesn't perform as well as it should is just the fact that Spigot doesn't have proper API implementation.

So you are not going to fix this bug that other amateur devs have managed to fix? Blaming it on spigot does not solve the issue, we all know that spigot's api is not so good but there is still a solution on your side.

For 1.13.2, the second glitch doesn't seem to happen, however, still on right click doesn't trigger most times for both versions, and the egg can still be right clicked when spammed

for 1.12.2 even this wont do the send message, because it doesn't listen to trigger. (Yes its this easy to reproduce just please copy my script and try it.)
on shoot:
if projectile is an egg:
send "&cTrigger worked"
cancel event

as for the second bug i originally posted, it is difficult to reproduce on an empty server with only skript. I think it might be other plugins or add ons that might break the event, I am not sure.

[Snow-Pyon]

So you are not going to fix this bug that other amateur devs have managed to fix? Blaming it on spigot does not solve the issue.

1. There is nothing to fix for an amateur developer, since there is no bug as this is expected behaviour for them, they just listen to other event when it is needed. What Skript does is listen to all the needed events to make your life simpler, I could make something like what Trademark posted above, and let you figure out what event and when you should use it making it utterly complex.

So, please don't come here to tell me "this is something anyone can fix" because you don't know what implications it has.

2. I haven't said it wasn't going to be fixed, I said it's upstream's fault, not Skript's one as you stated and thus, we gotta workaround it if possible.

I am trying to be nice here but if you're going to have that attitude, I may aswell close the issue and stop bothering about it.

[TsCode1]

I agree, I came off a little upset and I apologize for that

my request is for offhand to be fixed on right click, for the time being, the rest I give up on, so I am leaving it up for you guys if the whole cancel thing has issues or not, that is all. If I find a very easy way to reproduce my experienced issues and it is very clear then I will have to post it under a new issue because this is a messy thread.

[TheBentoBox]

I once again want to note that, on my live server using beta4 (so something could have broken since then), I am NOT having issues detecting or canceling offhand events. There are probably edge case situations which are either not firing click events (like the egg example above, which whether you understand it or not IS a Spigot problem) or which are detecting the wrong event-item, but in those scenarios there's probably always a different, better, more specific event you should be using instead.

[TsCode1]

I once again want to note that, on my live server using beta4 (so something could have broken since then), I am NOT having issues detecting or canceling offhand events. There are probably edge case situations which are either not firing click events (like the egg example above, which whether you understand it or not IS a Spigot problem) or which are detecting the wrong event-item, but in those scenarios there's probably always a different, better, more specific event you should be using instead.

Because it is only on right click event, and it is confirmed a bug. The off hand works fine for the other things I tested (except a few which I forgot, on projectile might be one of them). And there might be a better event to check, however, the bug still exists and we can all agree that the expected behavior does not occur.

[TheBentoBox]

We can't agree because I haven't verified and because our events are generally simple maps to Bukkit events so if Spigot isn't firing one in some situation then neither will Skript nor should it be expected to.

[TsCode1]

it is a bug regardless
(no more unnecessary commentary)