Skript
Skript copied to clipboard
SkriptLang
If "on teleport" is used, "on portal" stops working
Skript/Server Version
[15:51:47 Server thread/INFO] [Skript] Server Version: git-Paper-"aab1f8a" (MC: 1.18.2)
[15:51:47 Server thread/INFO] [Skript] Skript Version: 2.6.1
[15:51:47 Server thread/INFO] [Skript] Installed Skript Addons: None
[15:51:47 Server thread/INFO] [Skript] Installed dependencies: None
Bug Description
If you use on teleport event in any script, on portal event won't work.
Expected Behavior
Both events should be usable in any configuration.
Steps to Reproduce
on teleport:
stop trigger
on portal:
log "on portal"
message "on portal"
The above script will give no message when entering the portal. Remove the on teleport trigger, restart the server and you'll get the message. Without restarting the server, problem persists.
Errors or Screenshots
No response
Other
I've tested it on a clean 1.18.2 Paper server with only the newest Skript installed. No other scripts were present excluding the snippet above.
It was hard to reproduce, finding what prevented on portal from working when I had like 40 scripts was tedious, especially because it needed server restarts. It may be related to these issues which were closed because they couldn't be reproduced: #4331, #3006.
Agreement
- [X] I have read the guidelines above and affirm I am following them with this report.
On teleport event is called when the portal is about to teleport you, so cancelling on teleport will cancel all teleport causes e.g. nether protal cause
There's no cancelling of the event. stop trigger in my example code just does nothing. Actually on my server I had only some message shown to the player after teleporting, so nothing was done with the teleport itself.
If you replace stop trigger in on teleport with message "test" you'll get that message, but still no messages from on portal.
No, but the somewhat related issue from SkQuery was present for me for like two years, so if they have the same cause, it's an old bug.
Tested on 1.16.5 Paper, Skript 2.6.1
Tested using on teleport before and after on portal event
stop trigger only stopped teleport event from broadcasting "2"
Do you have any other plugins that might interfere with this?
As I've said, I've tested it on a clean server with only Skript. Are you testing on Paper?
But I've tested it on Paper... But only on 1.18.2. Paper introduces these async teleports etc., so I guess they may be some differences with that between spigot and paper.
Tested using skript-reflect and it works on both paper and spigot
import:
org.bukkit.event.player.PlayerTeleportEvent
org.bukkit.event.player.PlayerPortalEvent
on PlayerTeleportEvent:
# stop trigger
broadcast "2"
on PlayerPortalEvent:
broadcast "1"
no idea yet what caused it
With the code above, no restart was needed, I was able to switch between skript-reflect methods and skript's methods and former kept working while the latter failed from the first time
@oskarkk can you confirm the code above worked for you?
Also this works:
import:
org.bukkit.event.player.PlayerTeleportEvent
org.bukkit.event.player.PlayerPortalEvent
on teleport:
broadcast "2"
on PlayerPortalEvent:
broadcast "1"
And this doesn't:
import:
org.bukkit.event.player.PlayerTeleportEvent
org.bukkit.event.player.PlayerPortalEvent
on PlayerTeleportEvent:
broadcast "2"
on portal:
broadcast "1"
Seems like it may be an issue with the structure of the Event classes. PlayerPortalEvent (on portal) extends PlayerTeleportEvent (on teleport) extends PlayerMoveEvent (on any move). So basically, it seems like only the highest super of the events in the chain get called. This would explain why on any move from skquery breaks on teleport. It likely also breaks on portal. The reason skript-reflect doesn't have this issue is because it registers it's own listeners and the event that Skript sees is actually a wrapper class which shares no superclass with any Bukkit event.
I haven't tested but this check seem suspicious especially with the comment by it https://github.com/SkriptLang/Skript/blob/bc8458fccfc986f71ea4c36aaa02c64439d7f7e5/src/main/java/ch/njol/skript/SkriptEventHandler.java#L68-L69
This may also explain why the issue persists until you restart. It could be that the event listener is never unregistered, so the condition is still true.
It's implied in the Skript docs that Skript's on move event is never unregistered:
Called when a player or entity moves. NOTE: Move event will only be called when the entity/player moves position, not orientation (ie: looking around). NOTE: These events can be performance heavy as they are called quite often. If you use these events, and later remove them, a server restart is recommended to clear registered events from Skript.
The issue is that Skript only registered an event if there is not already a superclass of that event registered: https://github.com/SkriptLang/Skript/blob/31bc6c4d5b73fd055535b937e2698c10a086f37d/src/main/java/ch/njol/skript/SkriptEventHandler.java#L259-L274
However, this is not how Bukkit determines which listeners are called for which events. Bukkit determines this by the class that implements the static getHandlerList method (Bukkit stores listeners in an HanderList instance).
Because Skript first registers PlayerTeleportEvent, this adds the listener to the HandlerList of PlayerTeleportEvent. Now we get to the next trigger, which tries to register PlayerPortalEvent. However, Skript sees that it has already registered a superclass of this event (PlayerTeleportEvent), and it therefore doesn't register this. This is a problem, because when CB calls PlayerPortalEvent, its HandlerList does not contain Skript's listener.
This could be fixed by changing the register check to one that first gets the registration class (see SimplePluginManager#getRegistrationClass), checks if Skript has a listener registered for that class, and if not registers it (so the registeredEvents set should only contain events from getRegistrationClass).
We could also rework SkriptEventHandler, for example if we want it to unregister listeners, see for example reflect's, downside is that it'll unregister and re-register a listener when reloading.
