Skip to content

Events Migration (Player Events)#8716

Open
AnOwlBe wants to merge 25 commits into
SkriptLang:dev/featurefrom
AnOwlBe:Players
Open

Events Migration (Player Events)#8716
AnOwlBe wants to merge 25 commits into
SkriptLang:dev/featurefrom
AnOwlBe:Players

Conversation

@AnOwlBe

@AnOwlBe AnOwlBe commented Jun 20, 2026

Copy link
Copy Markdown

Problem

This is part 1 of probably 6-8 more prs needed to migrate all of simpleevents & njol/skript/events into the skriptlang folder in their related module while (hopefully) preventing any breaking changes.

This PR is targetted at player events in SimpleEvents (Events in Bukkit/Paper that start with Player I did not include every actual player event in SimpleEvents as thats too much for 1 pr)
Also migrates some of the events in njol/skript/events before realizing doing all of them in that folder would be too much ontop of this already big PR.

Why is this PR necessary? What problems exist that needed solving?
Everything worked fine so technically no problems but this needs to be done as SimpleEvents is pretty awful and very hard to read & manage & njol/skript/events is just simply running on deprecated registry

Events ->

  • EvtPlayerAnimation
  • EvtPlayerArmorChange
  • EvtPlayerBedEnter
  • EvtPlayerBedLeave
  • EvtPlayerBreakItem
  • EvtPlayerChangeBeaconEffect
  • EvtPlayerDamageItem
  • EvtPlayerDeepSleep
  • EvtPlayerElytraBoost
  • EvtPlayerEmptyBucket
  • EvtPlayerExprCooldownChange
  • EvtPlayerFillBucket
  • EvtPlayerFlightToggle
  • EvtPlayerHeldItem
  • EvtPlayerInventorySlotChange
  • EvtPlayerJoin
  • EvtPlayerJump
  • EvtPlayerKick
  • EvtPlayerLocaleChange
  • EvtPlayerLogin
  • EvtPlayerMendItem
  • EvtPlayerMoveOn
  • EvtPlayerQuit
  • EvtPlayerReadyArrow
  • EvtPlayerRespawn
  • EvtPlayerRiptide
  • EvtPlayerSneakToggle
  • EvtPlayerSpectate
  • EvtPlayerSprintToggle
  • EvtPlayerStopUsingItem
  • EvtPlayerSwapHandItems
  • EvtPlayerThrowEgg
  • EvtPlayerTrade
  • EvtPlayerWorldChange

Solution

Migrates every single player event from SimpleEvents into their own folder in the PlayerModule (some problems here i'll explain in a bit)

Chips away at the amount of lines in SimpleEvents & BukkitEventValues (progress to destroying the files!)

Testing Completed

Manual tested every single event with the event-values everything appears to work fine (besides from my bare bones testing some events) overall more testing will need to be completed before this is merged (ideally targetting skript 2.17 not 2.16)

Also ran JUnit & QuickTest both pass

Supporting Information

After finishing & even before it was pretty clear that theres an issue: Theres so many events that having em all in the PlayerModule in seperate files is too messy and too much
so (me and sovde) have a few ideas:
1# Put all events into files with categories like simpleevents but eg like PlayerBlockEvents or PlayerMiscEvents which would be on average like 200-300 lines per file containing events relating to the file name & any not fitting going in PlayerMiscEvents
2# Keep as is right now (mehhh decently messy)
3# Replicate the god awful simpleevents but smaller & do a PlayerEvents with all events in 1 file (please no)
4# Possibly another option?


Completes: none
Related: none
AI assistance: Minimal (Claude used to resolve issues with some events) otherwise all done by me

@AnOwlBe AnOwlBe requested a review from a team as a code owner June 20, 2026 05:56
@AnOwlBe AnOwlBe requested review from Burbulinis and Efnilite and removed request for a team June 20, 2026 05:56
@skriptlang-automation skriptlang-automation Bot added needs reviews A PR that needs additional reviews needs triage An issue that hasn't been classified or verified yet labels Jun 20, 2026
@skriptlang-automation

This comment has been minimized.

@AnOwlBe AnOwlBe changed the title Players Events Migration (Player Events) Jun 20, 2026
@AnOwlBe

AnOwlBe commented Jun 20, 2026

Copy link
Copy Markdown
Author

testing.txt
heres the code used for testing for those who want to test

Comment thread src/main/java/ch/njol/skript/events/SimpleEvents.java
@AnOwlBe

AnOwlBe commented Jun 20, 2026

Copy link
Copy Markdown
Author

also can someone add the 'don't merge' tag

@sovdeeth

Copy link
Copy Markdown
Member

As previously discussed, I think there's no reason for 80% of these to have their own files. It creates dozens of files that do nothing but return true and register the event. It would be best to just register the event via SimpleEvent in a PlayerEvents class and call it there. No need for thousands of lines of boilerplate. Organize them in a few methods by type, or whatever method fits best, but there's no need for 40 files in this pr.

@skriptlang-automation skriptlang-automation Bot added enhancement Feature request, an issue about something that could be improved, or a PR improving something. and removed needs triage An issue that hasn't been classified or verified yet labels Jun 20, 2026
@sovdeeth sovdeeth added don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. needs triage An issue that hasn't been classified or verified yet and removed enhancement Feature request, an issue about something that could be improved, or a PR improving something. labels Jun 20, 2026
@AnOwlBe

AnOwlBe commented Jun 20, 2026

Copy link
Copy Markdown
Author

As previously discussed, I think there's no reason for 80% of these to have their own files. It creates dozens of files that do nothing but return true and register the event. It would be best to just register the event via SimpleEvent in a PlayerEvents class and call it there. No need for thousands of lines of boilerplate. Organize them in a few methods by type, or whatever method fits best, but there's no need for 40 files in this pr.

Yeah I definitely saw that as I finished hence the question at the end:
image

@AnOwlBe

AnOwlBe commented Jun 20, 2026

Copy link
Copy Markdown
Author

should be ready for review now :)
(though no rush since this is planned for 2.17 probably)

AnOwlBe and others added 6 commits June 28, 2026 14:53
Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>
…nts/PlayerEvents.java

Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>
…nts/PlayerEvents.java

Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>
@AnOwlBe AnOwlBe requested a review from Absolutionism June 28, 2026 19:08
@AnOwlBe

AnOwlBe commented Jun 28, 2026

Copy link
Copy Markdown
Author

I changed pattern of chat to be [player] chat[ting] there shouldn't be any breaking changes but let me know if it should be reverted

@AnOwlBe AnOwlBe requested a review from Absolutionism July 2, 2026 03:43
…nts/events/EvtPlayerMoveOn.java

Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. needs reviews A PR that needs additional reviews needs triage An issue that hasn't been classified or verified yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants