-
Notifications
You must be signed in to change notification settings - Fork 2
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
[dispatcher] MSGHOOKS is kinda bad #48
Comments
And woah... as I just noticed modules capturing sendmsg events must never raise exception ever. (because default routine is to .say() exception stuff, and it keeps happening!) |
Seems like anything hooking outgoing messages shouldn't be using bot.say(); though generally for usefulness these types of hooks should be able to modify the message they're hooking, and yeah it should bail on exception (act as though the outgoing hook doesn't exist) if that's not terribly weird/complex/awful to implement. I need to acquaint myself better with how this is working before I can say more |
Some background information for this so when you look in to becoming more familiar with sendmsg event it might be clearer: In my mind I imagine there is 2 scenarios you want to implement a "sengmsg"
The commit I just pushed 248c8bd starts to realize that in that it allows the Mapping to either coexist, or fully override default behaviour. (steamchat module obviously does the former.) I hope this makes it a little bit clearer, and yes I agree with you that in the event that a module overriding the default behaviour somehow exceptions that a fallback to the internal sendmsg routine would be good, but it seems it would be non-trivial to implement. I have some initial ideas though. |
I currently rewrote how
sendmsg
andMSGHOOKS
works because I wanted to use sendmsg events in steamchat module, which made me realise that the current implementation was kind of bad. (The rewrite isn't commited/pushed yet.)Deciding how
Mapping.override
andMSGHOOKS
interact needs to be done before it can be implemented properly.So the only sane use of
sendmsg
at the moment is for capturing IRC bot outbound messages.The text was updated successfully, but these errors were encountered: