NachoSpigot icon indicating copy to clipboard operation
NachoSpigot copied to clipboard

PlayerInteractEvent abuse

Open andreasdc opened this issue 3 years ago • 14 comments

Observed Behavior

Standing on pressure plate when having PlayerInteractEvent cancelled results in massive spam of PlayerInteractEvents.

Expected Behavior

Normal calling of events.

Steps To Reproduce

  1. Cancel PlayerInteractEvent.
  2. Step on a pressure plate.

Plugin List

No response

Server Version

Other

No response

Agreements

  • [X] You were able to find this issue on the latest version of NachoSpigot.
  • [X] You have confirmed that there aren’t any issues open regarding this bug.
  • [X] You have confirmed that you are NOT using a fork of NachoSpigot in any way. YOUR changes are not OUR faults.

andreasdc avatar Dec 08 '21 03:12 andreasdc

Server Version

Why don't you put your server version?

ghost avatar Dec 08 '21 21:12 ghost

Server Version

Why don't you put your server version?

It's NachoSpigot, I thought it was obvious.

andreasdc avatar Dec 08 '21 21:12 andreasdc

Server Version

Why don't you put your server version?

It's NachoSpigot, I thought it was obvious.

NachoSpigot is the server, not the server version.

ghost avatar Dec 08 '21 21:12 ghost

Server Version

Why don't you put your server version?

It's NachoSpigot, I thought it was obvious.

NachoSpigot is the server, not the server version.

1.8.9

andreasdc avatar Dec 08 '21 21:12 andreasdc

1.8.9

By the server version I mean the version of NachoSpigot, and in the issue template it states this.

ghost avatar Dec 08 '21 21:12 ghost

1.8.9

By the server version I mean the version of NachoSpigot, and in the issue template it states this.

I thought this is irrelevant with this kind of issues.

andreasdc avatar Dec 08 '21 21:12 andreasdc

https://github.com/CobbleSword/NachoSpigot/blob/a2e26e2bce25888c37d08a73139d03da371e666f/NachoSpigot-Server/src/main/java/net/minecraft/server/BlockPressurePlateBinary.java#L51 We can't do much without a performance drop since it has to check if the pressure plate is powered.

@Lucaskyy Close as a wontfix?

ghost avatar Dec 08 '21 22:12 ghost

1.8.9

By the server version I mean the version of NachoSpigot, and in the issue template it states this.

I thought this is irrelevant with this kind of issues.

It's not :) We use it to identify on which commit you are. It's really handy for us, so it would be appreciated if you can add it if you create an issue.

Sculas avatar Dec 08 '21 22:12 Sculas

1.8.9

By the server version I mean the version of NachoSpigot, and in the issue template it states this.

I thought this is irrelevant with this kind of issues.

It's not :) We use it to identify on which commit you are. It's really handy for us, so it would be appreciated if you can add it if you create an issue.

This wasn't touched in any commit, so I don't really think that matters in this issue.

andreasdc avatar Dec 08 '21 22:12 andreasdc

https://github.com/CobbleSword/NachoSpigot/blob/a2e26e2bce25888c37d08a73139d03da371e666f/NachoSpigot-Server/src/main/java/net/minecraft/server/BlockPressurePlateBinary.java#L51

We can't do much without a performance drop since it has to check if the pressure plate is powered. @Lucaskyy Close as a wontfix?

It depends. Is Reflection invoking (caused by the event firing) better performant than a check for this? And can we stop the check from being spammed too?

Sculas avatar Dec 08 '21 22:12 Sculas

1.8.9

By the server version I mean the version of NachoSpigot, and in the issue template it states this.

I thought this is irrelevant with this kind of issues.

It's not :) We use it to identify on which commit you are. It's really handy for us, so it would be appreciated if you can add it if you create an issue.

This wasn't touched in any commit, so I don't really think that matters in this issue.

Even though it doesn't matter for this issue, it might for another issue. It's not that hard to add the version, so please next time just add it.

Sculas avatar Dec 08 '21 22:12 Sculas

It depends. Is Reflection invoking (caused by the event firing) better performant than a check for this? And can we stop the check from being spammed too?

Good point, I was just thinking because we'd have to save the last event and when it happened.

ghost avatar Dec 08 '21 22:12 ghost

Maybe we can do the check about registed listeners != 0 and call the event, if plugins aren't listening that event, it will not fire and will save cpu usage

ghost avatar Dec 09 '21 17:12 ghost

I'm going to make this an enhancement instead since it's not necessarily a bug.

Sculas avatar Dec 17 '21 19:12 Sculas