Add initial Discord integration support #99
Conversation
There was a problem hiding this comment.
Thanks for sending this in, as well as for your patience on the review!
Just as an FYI: I was planning to look into this feature after fixing #87 and #91, since those relate pretty heavily to config and initialization. Once I eventually do those, which I swear will happen one day, I will probably refactor a lot of this as well. This is plenty good enough for an initial pass on the feature, though.
Have you been able to verify that this works?
| } | ||
|
|
||
| type Discord struct { | ||
| // Token is the discord bot token |
There was a problem hiding this comment.
This must be the path to a file containing the token. Secrets do not live in the config.
| // Register as global commands | ||
| _, err := session.ApplicationCommandBulkOverwrite(event.Application.ID, "", commands) | ||
| if err != nil { | ||
| _ = fmt.Errorf("failed to update slash commands: %w", err) |
There was a problem hiding this comment.
Making an error and assigning it to _ does nothing. If it's ok for the program to live when this happens, then the error should be logged. If it isn't, then this error should be a panic.
| } | ||
| err := robo.brain.Forget(ctx, ch.Learn, event.Message.ID) | ||
| if err != nil { | ||
| _ = fmt.Errorf("failed to delete message: %w", err) |
|
|
||
| // Register message handler | ||
| session.AddHandler(func(session *discordgo.Session, event *discordgo.MessageCreate) { | ||
| robo.onDiscordMessage(ctx, session, event) |
There was a problem hiding this comment.
Using ctx here is suspicious because this callback outlives the scope the context is given to. Does discordgo.Session or discordgo.MessageCreate have a context.Context?
| if ch == nil { | ||
| return | ||
| } | ||
| err := robo.brain.Forget(ctx, ch.Learn, event.Message.ID) |
|
|
||
| // Start opens the Discord websocket connection. | ||
| func (mr *MessageReceiver) Start() error { | ||
| return mr.session.Open() |
There was a problem hiding this comment.
Probably a good idea to log that we're attempting to open the session at the start of this function.
|
|
||
| [discord] | ||
| # Discord bot token, requires the Message Content intent enabled. | ||
| token = '' |
There was a problem hiding this comment.
token = '$CREDENTIALS_DIRECTORY/discord_token'
| # however only use discord messages in discord due to how pings are | ||
| # formatted, emotes, etc... | ||
| # Other parameters work the same way as the twitch configurations. | ||
| configs = [ |
There was a problem hiding this comment.
Instead of an inline array, this should either use [[discord.configs]] syntax or work the same way as Twitch channel config.
|
|
||
| require ( | ||
| github.com/beorn7/perks v1.0.1 // indirect | ||
| github.com/bwmarrin/discordgo v0.28.1 // indirect |
| [twitch.bocchi.effects] | ||
| 'AAAAA' = 44444 | ||
|
|
||
| [discord] |
There was a problem hiding this comment.
Update config_test.go to test that this loads as expected.
I was having a chat with twin and this might be a fun (optional) addition, I saw you had a suggestion up for it on #7 so I gave it a shot. I'm a java guy not a golang guy so this is the best I could come up with and there's quite a lot of room for improvement.
This initial implementation should provide a means for robot to learn from a discord server as well as randomly post messages like she does in twitch, the configuration works in a slightly different way however. The idea was to be able to learn from both twitch and discord however only use the discord-learned messages on discord. It can also be setup to learn and work independently from twitch. It seems you kinda had that already with
LearnandSendtags, im not exactly sure how they work but I implemented/used them for this.I also implemented
sayandforget(slash) commands, she should also handle messages being deleted (by the user or a moderator).To deploy/connect to Discord just create a bot and enable the
Message Contentintent as that's necessary to read message contents. If a token is not specified it doesn't enable any discord integrations. It's necessary to create a server configuration for each discord server she's gonna be in, I think it'd be better to put it on a table instead of the toml file but for now this should work.Things you should probably check:
Things that could be improved:
Things that are not necessary:
Things that need to be looked into eventually:
@the user but not cause a notification)Theres quite a lot of things that could be generalized/decoupled from the IRC client, such as learn and think calls to reduce all the duplicate code I ended up introducing with the discord methods. Generalizing some of the meme/copypasta/block/trace and other similar things could also go a long way now that this exists.
I'm assuming that to your golang eyes this is garbo code (I have no clue how that
robo *Robotparameter gets passed around) so feel free to tell me what to fix or do any changes you feel are fit. You can also message me on discord if you want to go over anything. I did test this so the bare minimum works.