Skip to content

flexible listener priorities#1504

Open
ieatglu3 wants to merge 4 commits into
retrooper:2.0from
ieatglu3:re-flexible-listener-priorities
Open

flexible listener priorities#1504
ieatglu3 wants to merge 4 commits into
retrooper:2.0from
ieatglu3:re-flexible-listener-priorities

Conversation

@ieatglu3
Copy link
Copy Markdown

This rewrites the listener priority system in a way that is more robust, including supporting custom integer priorities.

For maximum backwards compatibility, this does not remove the legacy PacketListenerPriority, nor its usages, instead just deprecates it without sacrificing existing functionality.

The modern listener priority class is now ListenerPriority, with the same default/predefined priority values as its legacy counterpart.

User's can continue to use existing priorities, or create their own using the new system.

@ieatglu3 ieatglu3 changed the title rewrite listener priority system flexible listener priorities Apr 25, 2026
@rafi67000
Copy link
Copy Markdown
Contributor

Why not reuse the PacketListenerPriority?

@ieatglu3
Copy link
Copy Markdown
Author

ieatglu3 commented Apr 26, 2026

Why not reuse the PacketListenerPriority?

This is a breaking change; the class type, and structure would have changed. This enum has remained the same since 1.0 (5 years). Unless breaking plugins... Isn't a concern? Then sure, It would make more sense to reuse it.

@booky10
Copy link
Copy Markdown
Collaborator

booky10 commented Apr 28, 2026

@ieatglu3 can you activate this? want to push a few changes + formatting fixes
image

@ieatglu3
Copy link
Copy Markdown
Author

@ieatglu3 can you activate this? want to push a few changes + formatting fixes

image

there

@booky10
Copy link
Copy Markdown
Collaborator

booky10 commented Apr 28, 2026

Thanks

Comment on lines +65 to +67
// no point in providing a backwards compat if method returns the modern type
// + the new type has to be exposed in one way or another (the event manager needs it), cant get around this one :(
return this.priority;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// no point in providing a backwards compat if method returns the modern type
// + the new type has to be exposed in one way or another (the event manager needs it), cant get around this one :(
return this.priority;
// backwards compat, developers may have overrides
return ListenerPriority.fromLegacy(this.getPriority());

this needs to check for overrides of #getPriority()

Copy link
Copy Markdown
Author

@ieatglu3 ieatglu3 Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bro?? This always just returns a predefined value; custom priorities become useless. Even then, this Is an overcompensation, If someone overrode #getPriority, then this just means their legacy override would be ignored in favour for what was passed into the constructor. The priority has always been final meaning if someone overrode it's !!! getter !!! #getPriority, the override inherently becomes unsafe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants