From 831c708c78a9f356a0cedf30611eb642b0799f7f Mon Sep 17 00:00:00 2001 From: Timothy du Heaume <timothy.duheaume@gmail.com> Date: Tue, 4 Feb 2020 10:35:51 +0900 Subject: [PATCH] refactor sync_all_role_reactions (was "add_all_role_reactions") --- src/main.rs | 4 ++-- src/reaction_roles.rs | 36 ++++++++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0711249..348aa62 100644 --- a/src/main.rs +++ b/src/main.rs @@ -188,11 +188,11 @@ impl EventHandler for Handler { // In this case, just print what the current user's username is. fn ready(&self, ctx: Context, ready: Ready) { info!("{} is connected!", ready.user.name); - reaction_roles::add_all_role_reactions(ctx); + reaction_roles::sync_all_role_reactions(ctx); } fn resume(&self, ctx: Context, _: serenity::model::event::ResumedEvent) { - reaction_roles::add_all_role_reactions(ctx); + reaction_roles::sync_all_role_reactions(ctx); } } diff --git a/src/reaction_roles.rs b/src/reaction_roles.rs index 8add1b4..75c0e93 100644 --- a/src/reaction_roles.rs +++ b/src/reaction_roles.rs @@ -2,7 +2,7 @@ use crate::config::CONFIG; use crate::util::{get_react_from_string, get_string_from_react}; use serenity::{ client::Context, - model::{channel::Message, channel::Reaction, id::UserId}, + model::{channel::Message, channel::Reaction, id::UserId, id::RoleId}, }; use std::collections::{HashMap, HashSet}; use std::iter::FromIterator; @@ -39,7 +39,7 @@ pub fn remove_role_by_reaction(ctx: Context, msg: Message, removed_reaction: Rea }); } -pub fn add_all_role_reactions(ctx: Context) { +pub fn sync_all_role_reactions(ctx: Context) { let messages_with_role_mappings = get_all_role_reaction_message(&ctx); let guild = ctx.http.get_guild(CONFIG.server_id).unwrap(); // this method supports paging, but we probably don't need it since the server only has a couple of @@ -50,6 +50,9 @@ pub fn add_all_role_reactions(ctx: Context) { .get_guild_members(CONFIG.server_id, Some(1000), None) .unwrap(); + let mut roles_to_add: HashMap<UserId, Vec<RoleId>> = HashMap::from_iter(all_members.iter().map(|m| (m.user_id(), Vec::new()))); + let mut roles_to_remove: HashMap<UserId, Vec<RoleId>> = HashMap::from_iter(all_members.iter().map(|m| (m.user_id(), Vec::new()))); + for (message, mapping) in messages_with_role_mappings { for (react, role) in mapping { // the docs say this method can't retrieve more than 100 user reactions at a time, but it seems @@ -61,18 +64,31 @@ pub fn add_all_role_reactions(ctx: Context) { .unwrap(); let reactor_ids: HashSet<UserId> = HashSet::from_iter(reactors.iter().map(|r| r.id)); - // this looks O(n!), but n will probably never be more than three digits, so maybe it's okay? - // one solution might be to batch up all the roles to add/remove for each member and do them - // all at once with .add_roles() - for mut member in all_members.clone() { - if reactor_ids.contains(&member.user_id()) { - member.add_role(ctx.http.clone(), role).unwrap(); - } else { - member.remove_role(ctx.http.clone(), role).unwrap(); + for member in all_members.clone() { + let user_id = &member.user_id(); + if reactor_ids.contains(&user_id) { + if !member.roles.iter().any(|r| r == role) { + roles_to_add.get_mut(&user_id).unwrap().push(*role); + } + } else if member.roles.iter().any(|r| r == role) { + roles_to_remove.get_mut(&user_id).unwrap().push(*role); } } } } + + for (user_id, roles) in roles_to_add { + if !roles.is_empty() { + let mut member = all_members.iter().find(|m| m.user_id() == user_id).unwrap().clone(); + member.add_roles(ctx.http.clone(), &roles[..]).unwrap(); + } + } + for (user_id, roles) in roles_to_remove { + if !roles.is_empty() { + let mut member = all_members.iter().find(|m| m.user_id() == user_id).unwrap().clone(); + member.remove_roles(ctx.http.clone(), &roles[..]).unwrap(); + } + } } fn get_all_role_reaction_message( -- GitLab