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

Start using 2 Dispatcher classes #214

Open
TAG-Epic opened this issue Mar 25, 2023 · 2 comments
Open

Start using 2 Dispatcher classes #214

TAG-Epic opened this issue Mar 25, 2023 · 2 comments
Labels
scope/nextcore.common Any changes to the nextcore.common namespace type/feature A feature addition

Comments

@TAG-Epic
Copy link
Member

Currently there is only 1 Dispatcher - The Dispatcher.

It has no specific typing except for the event name, other than that its *args: Any, **kwargs: Any which kind of makes sense for gateway opcode events & gateway DISPATCH events. (as manually filtering this yourself is stupid)

I don't have any specific ideas for improvement for opcode events and DISPATCH events, however for events nextcore produces, using vauge and dynamic typing is stupid.

I propose something like this (No clue about class name

d: Dispatcher2[EventType1, EventType2] = Dispatcher2()

@d.listen()
async def all_events(event: EventType1 | EventType2): # Or just realistically use typing.Any
    ...
@d.listen(EventType1)
async def one_event(event: EventType1):
    ...

This avoids reading typehints - which I do not want in this library, as it is "magic".

@TAG-Epic TAG-Epic added scope/nextcore.common Any changes to the nextcore.common namespace type/feature A feature addition labels Mar 25, 2023
@EmmmaTech
Copy link

What about calling the new dispatcher TypedDispatcher since it has more specific typing? Alternatively, the current dispatcher could be renamed RawDispatcher, but that would obviously be a breaking change.

@TAG-Epic
Copy link
Member Author

imo EventDispatcher is more accurate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope/nextcore.common Any changes to the nextcore.common namespace type/feature A feature addition
Projects
None yet
Development

No branches or pull requests

2 participants