CobbleSword / NachoSpigot

NachoSpigot is a fork of TacoSpigot 1.8.9 that offers several enhancements to performance as well as bug fixes.
GNU General Public License v3.0
237 stars 87 forks source link

PlayerInteractEvent abuse #280

Open andreasdc opened 2 years ago

andreasdc commented 2 years ago

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

ghost commented 2 years ago

Server Version

Why don't you put your server version?

andreasdc commented 2 years ago

Server Version

Why don't you put your server version?

It's NachoSpigot, I thought it was obvious.

ghost commented 2 years ago

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.

andreasdc commented 2 years ago

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

ghost commented 2 years ago

1.8.9

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

andreasdc commented 2 years ago

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.

ghost commented 2 years ago

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?

Sculas commented 2 years ago

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.

andreasdc commented 2 years ago

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.

Sculas commented 2 years ago

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 commented 2 years ago

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.

ghost commented 2 years ago

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 commented 2 years ago

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

Sculas commented 2 years ago

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