Some 2.0 improvements for event handlers

[ioistired]

A class per event type

I'm thinking about how dpy 2.0 is planned to use a single event payload object for all events. I noticed that Telethon does a similar thing:

@bot.on(events.NewMessage())
async def on_message(event: events.NewMessage.Event):
    await event.reply('Hi there')

Here events.NewMessage implements a base class for all event types.

We don't need the constructor parameters for NewMessage so we can get rid of the nested class too, which, in d.py might look like this:

@bot.listen(events.NewMessage)
async def on_message(event: events.NewMessage):
    await event.reply('Hi there')

(It's also assumed that by the time 2.0 is finalized, so will Discord's new reply system 😛)

I like this approach for a few reasons:

• It makes event handlers more introspectable. We could wrap event handlers in a class like was done for Command, and then instead of a string to represent the event, we could offer various attributes on the handler, such as a list of all its checks, and what event type it's listening for.
• It lets us properly namespace event types. Bot.dispatch is currently undocumented because namespace collisions between dpy events and user-defined events are undesired. Classes are namespaced by their module which neatly solves this issue.
• It may make typing stricter. Bot.listen could have a signature specified as follows:

from __future__ import annotations

from dataclasses import dataclass

from abc import ABCMeta
from typing import *

class User: pass
class Member: pass
class Guild: pass

class BaseEvent(metaclass=ABCMeta):
    ...

T = TypeVar('T', bound=BaseEvent)
TT = TypeVar('TT', bound=Type[BaseEvent])

Check = Callable[[T], bool]

class Message(BaseEvent):
    author: Union[User, Member]
    content: Optional[str]
    ...

class GuildLeave(BaseEvent):
	guild: Guild

HandlerCallback = Callable[[T], Awaitable[None]]

@dataclass
class EventHandler(Generic[TT]):
    checks: List[Check]
    callback: HandlerCallback[TT]

class Bot:
	def listen(self, t: TT) -> Callable[[HandlerCallback], EventHandler[TT]]:
		def deco(cb: HandlerCallback) -> EventHandler[TT]:
			self.add_listener(t, cb)
			return EventHandler(checks=[], callback=cb)
		return deco

	def add_listener(self, t: TT, cb: HandlerCallback) -> None: ...

bot = Bot()
@bot.listen(Message)
async def on_message(event: Message): ...

Sadly event: GuildLeave also passes mypy on the last line, so eh, this isn't too worth considering.

• It would let us store additional attributes for all events. For example, an optional debug mode, disabled by default, might store the unparsed socket response accessible by BaseEvent.raw_data. Or we could decide to have BaseEvent take parameters like include_socket_response=True in order to allow event handlers to opt-in to using undocumented or unsupported gateway features.

Checks for events

Features such as @commands.is_owner do not need to be unique to Commands. Caveat: I do not have any good motivating use cases for reusable checks on event handlers.

[Rapptz]

I'm not really a fan of using checks along side with events. The use cases for it are rather slim in my opinion and don't add too much compared to actually inlining it with the callback body. If someone had something that was done repeatedly in the callback body then they are free to use an actual pure decorator to do this.

Part of the reason I'm subscribing to this event object design is a reason not mentioned: it allows me to move some of this stuff over to native code where it's easier to construct these objects and blast them to Python. Adding checks into the mix for this complicates that design space a bit more for me.

Passing the class to the decorator rather than operating on using names constitutes as a rather hard breaking change that would be non-trivial to migrate to. While I am a proponent of doing breaking changes I also don't want to overly break all clients. This approach would also need to be adapted for the events defined within the class pattern which I think is better and allows for cleaner code. The name approach allows for seamless overriding of defaults, for example a user right now is able to override on_message inside commands.Bot for more advanced handling since it overrides the actual function name. If I move over to a different system where, for example, Bot.listen semantics become the default rather opt-in via commands.Bot then this problem of being able to override the default event listener would have to be thought over.

[ioistired]

To override one could use a different decorator as is done now, which instead of setting an attribute on the bot, just replaces a single value in a dict. This could mean a Dict[Type[EventType], Tuple[Optional[EventHandler], List[EventHandler]]] where the first element of the tuple is the "default" handler and the list is the current value type of _extra_events