Make the bot follow multiple users #32
Conversation
Signed-off-by: baalajimaestro <[email protected]>
Signed-off-by: baalajimaestro <[email protected]>
Signed-off-by: baalajimaestro <[email protected]>
Signed-off-by: baalajimaestro <[email protected]>
Signed-off-by: baalajimaestro <[email protected]>
Signed-off-by: baalajimaestro <[email protected]>
Signed-off-by: baalajimaestro <[email protected]>
| let mut channel_id = id::ChannelId(0); | ||
| for i in channel_ids { | ||
| if i.is_some() { | ||
| // Choose the first channel we encounter which is non-null |
There was a problem hiding this comment.
Your comment doesn't match your code. You are taking the last channel which is some. Later loop iterations are overwriting your channel_id.
An early break would make sense here.
There was a problem hiding this comment.
See comments above / below, this extra loop should not be necessary at all.
| // Check if any of the admins are in the VC | ||
| let user_list = guild.voice_states; | ||
|
|
||
| let mut channel_ids: Vec<Option<id::ChannelId>> = Vec::new(); |
There was a problem hiding this comment.
Why do you use here an vector if in fact you only need one single channel_id later on?
Add the is_some check of line 142 as a condition to the if in 130 and only assign if a valid channel was found.
The var channel_id should then be an Option<id::ChannelId> initialized to None.
Then you can check if it is still None after your loop, if that's the case log that the 'admin' was not found. If it is some, join the channel.
| // Check if any of the admins are in the VC | ||
| let user_list = guild.voice_states; | ||
|
|
||
| let mut channel_ids: Vec<Option<id::ChannelId>> = Vec::new(); |
There was a problem hiding this comment.
You only need one channel_id later on. Just use an Option<id::ChannelId> and move the is_some() check into the loop in line 71.
There was a problem hiding this comment.
You don't need to store it at all, just connect directly if a valid channel is found (and then break).
| for i in channel_ids { | ||
| if i.is_some() { | ||
| // Enable casting | ||
| player.lock().await.enable_connect().await; |
There was a problem hiding this comment.
Break here to prevent multiple connect attempts.
| @@ -43,7 +43,7 @@ services: | |||
| - DISCORD_TOKEN= | |||
| - SPOTIFY_USERNAME= | |||
| - SPOTIFY_PASSWORD= | |||
| - DISCORD_USER_ID= # Discord user ID of the user you want Aoede to follow | |||
| - DISCORD_ADMINS= # List (wrapped in single quotes) of users u want the bot to keep track of, eg value: '["100","200"]' | |||
There was a problem hiding this comment.
Personally I don't like the name DISCORD_ADMINS. Someone could think that he must list the discord server admins / owners here. Why not DISCORD_USERS?
|
Thanks for the quick review, will resolve the requested changes |
|
Did any feature like this ever get merged into the main branch? |
Fixes #31
Tested to be working just fine
Required breaking change:
DISCORD_USER_IDis nowDISCORD_ADMINSwhich is a listFor supplying via env vars,
DISCORD_ADMINS = '["user1","user2"]'