Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skript highest priority setting causes Skript commands to be undetected in command event #5908

Closed
1 task done
not-3than opened this issue Aug 13, 2023 · 8 comments
Closed
1 task done
Assignees
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.

Comments

@not-3than
Copy link

Skript/Server Version

[02:39:11 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[02:39:11 INFO]: [Skript] Skript's documentation can be found here: https://skriptlang.github.io/Skript
[02:39:11 INFO]: [Skript] Skript's tutorials can be found here: https://docs.skriptlang.org/tutorials
[02:39:11 INFO]: [Skript] Server Version: git-Paper-388 (MC: 1.18.2)
[02:39:11 INFO]: [Skript] Skript Version: 2.6.4
[02:39:11 INFO]: [Skript] Installed Skript Addons: 
[02:39:11 INFO]: [Skript]  - skript-placeholders v1.5.2 (https://github.com/APickledWalrus/skript-placeholders)
[02:39:11 INFO]: [Skript]  - skript-gui v1.3 (https://github.com/APickledWalrus/skript-gui)
[02:39:11 INFO]: [Skript]  - DiSky v4.7.0
[02:39:11 INFO]: [Skript]  - MorkazSk v1.3
[02:39:11 INFO]: [Skript]  - skript-reflect v2.3 (https://github.com/TPGamesNL/skript-reflect)
[02:39:11 INFO]: [Skript]  - skRayFall v1.9.28 (https://sk.rayfall.net/)
[02:39:11 INFO]: [Skript]  - SkBee v2.16.0 (https://github.com/ShaneBeee/SkBee)
[02:39:11 INFO]: [Skript] Installed dependencies: 
[02:39:11 INFO]: [Skript]  - Vault v1.7.3-b131
[02:39:11 INFO]: [Skript]  - WorldGuard v7.0.7+216b061

Bug Description

Recently I set the plugin priority of Skript to highest to see if it could detect WorldGuard blocking on break: events, but forgot to switch it back. I have a few scripts (including a combatlog one) that check to see if a player is running a command via on command: event. After noticing that it couldn't detect any of my custom teleportation commands, I further investigated and found it couldn't detect any Skript commands at all. It wasn't until I changed my priority back to high was I able to see those commands in the command event.

Expected Behavior

Any and all commands a player runs should fire the on command: event, but for some reason in this case with this particular version (haven't tested others), it would not fire for Skript commands (except for /skript + aliases).

Steps to Reproduce

  1. Use Paper 1.18.2 and Skript 2.6.4
  2. Set priority to highest in config.sk
  3. Use on command: to detect commands, and try a command created in Skript.

Errors or Screenshots

No response

Other

No response

Agreement

  • I have read the guidelines above and affirm I am following them with this report.
@AyhamAl-Ali
Copy link
Member

See this comment for more info on plugin priorities however, it may not be very related to this specific issue. Other plugins may have cancelled some/all commands in certain situations and with Skript being highest priority makes Skript ignore the cancelled events, this is mostly fixed in #5896

@AyhamAl-Ali AyhamAl-Ali added the investigating The core developers are currently investigating this issue. Usually used for complex cases. label Aug 13, 2023
@sovdeeth
Copy link
Member

sovdeeth commented Sep 3, 2023

After preliminary investigation:
Skript handles commands a bit strangely, due to the necessity of being able to register and deregister on the fly.
Namely, it uses the PlayerPreprocessCommandEvent to listen for its commands, rather than doing the proper registration hullabaloo. It listens to this event on HIGHEST, the latest possible moment it can (and still cancel the event). It checks if the attempted command matches a Skript event, and if so, cancels the event so other plugins don't try to handle it.
However, the on command event ALSO listens to PPCE, and if it's at HIGHEST or MONITOR, the event will be cancelled by Skript by the time it gets around to running, leading the to Skript trigger never being called.
I'll look into this further to see if there's any workaround, but I think it may be something we just have to live with.

@sovdeeth sovdeeth added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. labels Sep 3, 2023
@not-3than
Copy link
Author

Alright thank you! It does seem to be quite a hinderance on overall functionality, seeing as how the majority of that event no longer functions. Please let me know if there’s something I can do to fix this!

@sovdeeth
Copy link
Member

sovdeeth commented Sep 3, 2023

Alright thank you! It does seem to be quite a hinderance on overall functionality, seeing as how the majority of that event no longer functions. Please let me know if there’s something I can do to fix this!

The only thing you can do is make sure the on command event runs at HIGH priority or lower, like with priority HIGH.

@sovdeeth sovdeeth added PR available Issues which have a yet-to-be merged PR resolving it and removed investigating The core developers are currently investigating this issue. Usually used for complex cases. labels Sep 3, 2023
@sovdeeth
Copy link
Member

sovdeeth commented Sep 3, 2023

@not-3than Try this nightly jar for 2.7: https://github.com/SkriptLang/Skript/actions/runs/6066909722
I can make you one for 2.6.4 if you can't test on 2.7 for some reason.

@not-3than
Copy link
Author

not-3than commented Sep 3, 2023 via email

@sovdeeth
Copy link
Member

sovdeeth commented Sep 3, 2023

I have a live server and try not to use dev versions. If you can make 2.6.4 I can test. This isn’t a huge issue for me, I didn’t end up needing to change priorities in skript config, but I can still test nonetheless.- Ethan

This jar should work for 2.6.4
Skript.zip

@sovdeeth
Copy link
Member

This issue currently only exists when listening at HIGHEST or MONITOR for commands that are run by players that do not have the permission to use them. To my knowledge, fixing this completely will require a breaking change to how permission messages are sent (see the above PR for more detail, 6126).
We'll leave this open until that change is made (2.8?) or some better solution comes along.

@sovdeeth sovdeeth removed the PR available Issues which have a yet-to-be merged PR resolving it label Oct 17, 2023
@UnderscoreTud UnderscoreTud added the PR available Issues which have a yet-to-be merged PR resolving it label Dec 23, 2023
@sovdeeth sovdeeth removed the PR available Issues which have a yet-to-be merged PR resolving it label Jan 2, 2024
@sovdeeth sovdeeth added PR available Issues which have a yet-to-be merged PR resolving it feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Apr 7, 2024
@APickledWalrus APickledWalrus added completed The issue has been fully resolved and the change will be in the next Skript update. and removed PR available Issues which have a yet-to-be merged PR resolving it feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.
Projects
None yet
Development

No branches or pull requests

5 participants