Event interfaces
s1kx opened this issue · 1 comments
As of now, event handling has been improved with the events
subpackage, but actual usage is still not pretty.
Event hooks still have to deal with Event interface{}
in DiscordEvent
and then do type assertions and type casting (in case they are subscribed to multiple events). Needless to say, things like this are not idiomatic:
func onChatlogEvent(ctx *unison.Context, dv *events.DiscordEvent) (bool, error) {
var m *discordgo.Message
// Check event type
switch ev := dv.Event.(type) {
default:
return false, nil
case *discordgo.MessageCreate:
m = ev.Message
}
// ...
}
Instead, I want to propose creating interfaces for groups of related events.
If you look at the event/types.go, you see that there are a lot of events that are related to the same subject (MessageCreate/Update/Delete).
Most of these events in discordgo also only embed the underlying struct (e.g. struct MessageCreate { *Message }
). Some discordgo events also don't contain a timestamp, which might be useful in cases where actual event processing is delayed.
To improve the event hook API and making the code more testable, I believe that we should:
- Rename current types in the
events
package, so thatFooEvent
and every other exported type ending withEvent
will be an interface - Create an interface for all discord event classes rather than exporting the current
DiscordEvent
wrapper type - Add timestamps to all events and make all event interfaces export an
Action()
orOperation()
method that reflects the nature of the event
Example code:
type discordEvent struct {
// current DiscordEvent code here
// ReceivedAt is the timestamp when the event was received
ReceivedAt time.Time
}
type DiscordEvent interface {
// Type returns the type identifier of the event
Type() EventType
// Event returns the discordgo event, which has to be typecasted for usage
Event() interface{}
}
// PersistenceAction is an enum reprenting the default actions covered by most event groups
type PersistenceAction int
const (
CreateAction PersistenceAction = iota
UpdateAction
DeleteAction
)
type MessageEvent interface {
DiscordEvent
Action() PersistenceAction
Message() *discordgo.Message
}
Note:
Type-specific events such as MessageEvent
should probably still define their own Action type.
However, this will lead to problems comparing e.g. PersistenceAction
values and MessageAction
values.
Whether it's worth having an Action()
method I am not entirely sure about yet, since the action is already clear from Type()
. There might be benefit to having a Action()
reside in DiscordEvent
and define ALL possible actions in a single enum. Opinions please!
Now the EventHook
type can be adjusted to contain callbacks for any class of events, and the Events
list will be made obsolete, since EventDispatcher
can now simply check if a callback exists:
// EventHook is an application-level type for handling discord requests.
// All callbacks are optional, and whether they are defined or not
// is used to determine whether the EventDispatcher will send events to them.
type EventHook struct {
// current EventHook fields here
// OnEvent is called for all events.
// Handlers must typecast the event type manually, and ensure
// that it can handle receiving the same event twice if a type-specific
// callback also exists.
OnEvent func(ctx *Context, ev events.DiscordEvent) error
// OnMessageEvent is called for every message-related event.
OnMessageEvent func(ctx *Context, ev events.MessageEvent) error
// OnConnectionEvent ...
// OnUserEvent ...
// OnChannelEvent ...
// OnGuildEvent ...
}
This will simplify most use-cases of event hooks, avoiding type assertions and type casting when not explicitely required.
The trade-off here is that EventHooks will be fired on actions they might not want to handle. I think the performance impact would be minimal however and worth the improvements to the packages API for users and testability.
While on the subject, now might be a good time to consider also moving all event logic into the subpackage. Seems like the hooks and dispatcher logically belong there, building one logical unit, separate from the chatbot core.
The only dependency back to the rest of the project is the Context parameter in the callbacks. We could either change this to ctx interface{}
, or use variadic parameters (as in OnEvent func(ev events.DiscordEvent, args ...interface{}) error
.