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

Confusion between registerEvent() and registerEvents() #2796

Open
Ifera opened this issue Mar 3, 2019 · 8 comments
Open

Confusion between registerEvent() and registerEvents() #2796

Ifera opened this issue Mar 3, 2019 · 8 comments
Labels
BC break Breaks API compatibility Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP

Comments

@Ifera
Copy link
Contributor

Ifera commented Mar 3, 2019

Description

It has long been a problem especially to the people who are new in PocketMine field between registerEvent() and registerEvents().

That one addition of s makes it a completely different function from the other. The reason for that is the confusing/conflicting names given to both these methods.

Alternative methods:

Following changes could be made to solve this confusion.

  • registerEvents() should be changed to registerListener().
  • registerEvent() should be changed to registerHandler().
@Ifera Ifera changed the title Confusion between registerEvent() and regusterEvents() Confusion between registerEvent() and registerEvents() Mar 3, 2019
@dktapps dktapps added Category: API Related to the plugin API BC break Breaks API compatibility Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Mar 3, 2019
@dktapps dktapps added this to the 4.0 milestone Mar 3, 2019
@95CivicSi
Copy link
Contributor

I know that I struggled with this when I first started updating and writing plugins. I agree that this change would read better and make each function more transparent about what it does.

@dktapps
Copy link
Member

dktapps commented Mar 4, 2019

Personally I was far more confused by onCommand() but that's a topic for a separate issue.

@InspectorGadget
Copy link

Now, this is gonna break plugins :D

@dries-c
Copy link
Contributor

dries-c commented Mar 4, 2019

It's only one change lol. Wait till 4.0 hit the bell lol

@SOF3
Copy link
Member

SOF3 commented Mar 4, 2019

I knew it. https://twitter.com/SOFe1970/status/1001841400416108544

@dktapps
Copy link
Member

dktapps commented Mar 8, 2019

bUkKiT hAs aN OuTstAnDinG aPi deSiGn

@dktapps dktapps modified the milestones: 4.0, 5.0 Oct 11, 2021
@Gewinum
Copy link

Gewinum commented Jul 7, 2022

better then rename it to
registerEvent => registerEventHandler
registerEvents => registerEventsListener

i think, it becomes more understandable than just "registerHandler" and "registerListener", idk. if we think about fact, that these functions are in «PluginManager»

@dktapps
Copy link
Member

dktapps commented Jul 7, 2022

if we think about fact, that these functions are in «PluginManager»

yes, indeed

@dktapps dktapps removed this from the 5.0 milestone Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC break Breaks API compatibility Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

No branches or pull requests

7 participants