UQComputingSociety / uqcsbot-discord

:mortar_board: UQCSbot: Our friendly little Discord bot
https://discord.uqcs.org
MIT License
20 stars 20 forks source link

fix broken yelling exemptor #162

Closed andrewj-brown closed 1 year ago

andrewj-brown commented 1 year ago

The yelling exemptor breaks literally any command that uses self. This includes bgg, cowsay, hoogle, manage_cogs, mcwhitelist, voteythumbs, and zalgo.

This is the sort of issue that pyright should be able to pick up (and in fact, this PR currently shouldn't pass a typecheck, because every time you apply the yelling exemptor, it unthinkingly casts commands.Cog down to UQCSBotCog). The fact that it doesn't concerns me and I may have to investigate pyright configs to work out why this is the case.

andrewj-brown commented 1 year ago

Bradley I'd like your review purely because I'm unsure why you used bot as the first argument to begin with; is there something more obvious I've missed?

bradleysigma commented 1 year ago

The cogself was extracted to allow access to the .bot object, which is used for bot.emojis and external_handle_bans.

FWIW, those commands were working at some point on the testing server. image

Have you tested the changed commands in #yelling?

andrewj-brown commented 1 year ago

I don't disagree with your extraction of the bot for use in bot.emojis or handling external bans, but the problem is every time you call func you're providing bot as the first argument. Those functions are all defined by Cog subclasses, and expect a Cog as their self argument, not a Bot.

Obviously this behaviour doesn't trigger in the case when the message should be blocked (evident in your screenshot), because that's the only case that doesn't go on and call func.

bradleysigma commented 1 year ago

Oh, right, that was because originally I had made bot the first argument to wrapper, and so passed it to func. Later I realised it was actually the COG, and so added in cogself, but forgot to update the calls to func. Whoops.

andrewj-brown commented 1 year ago

Ah, sweet. You had me worried that there was something truly scuffed going on in the background that I didn't know about.

andrewj-brown commented 1 year ago

In that case, I'll merge now, so we regain access to the (definitely very widely used) broken commands.