From 60fa8f40ec2b9be807da5dfb4aeef18f4a6bbbfd Mon Sep 17 00:00:00 2001 From: Noikoio Date: Fri, 7 Jun 2019 16:46:12 -0700 Subject: [PATCH] Improved role processing Made the process more efficient so it iterates over less names as it determines which roles to alter. Additionally, it now attempts to be proactive in finding potential issues when setting roles and skipping them if there are issues. This had been causing issues with rate limiting due to having an enormous number of failed requests. --- BirthdayBot/BackgroundWorker.vb | 95 ++++++++++++++++++++++++--------- BirthdayBot/BirthdayBot.vbproj | 4 +- 2 files changed, 71 insertions(+), 28 deletions(-) diff --git a/BirthdayBot/BackgroundWorker.vb b/BirthdayBot/BackgroundWorker.vb index 1c22e78..0fb653a 100644 --- a/BirthdayBot/BackgroundWorker.vb +++ b/BirthdayBot/BackgroundWorker.vb @@ -120,29 +120,56 @@ Class BackgroundWorker Dim birthdays = BirthdayCalculate(users, tz) ' Note: Don't quit here if zero people are having birthdays. Roles may still need to be removed by BirthdayApply. - ' Set birthday role, get list of users now having birthdays + ' Set birthday roles, get list of users that had the role added + ' But first check if we are able to do so. Letting all requests fail instead will lead to rate limiting. Dim announceNames As IEnumerable(Of SocketGuildUser) - Try - announceNames = Await BirthdayApplyAsync(guild, role, birthdays) - Catch ex As Discord.Net.HttpException - If ex.HttpCode = HttpStatusCode.Forbidden Then - SyncLock _bot.KnownGuilds - ' Failed to apply role. Set the warning. - _bot.KnownGuilds(guild.Id).RoleWarning = True - End SyncLock - Return 0 - End If + If BirthdayHasGoodRolePermissions(guild, role) Then + Try + announceNames = Await BirthdayApplyAsync(guild, role, birthdays) + Catch ex As Discord.Net.HttpException + If ex.HttpCode = HttpStatusCode.Forbidden Then + announceNames = Nothing + Else + Throw + End If + End Try + Else + announceNames = Nothing + End If + + If announceNames Is Nothing Then + SyncLock _bot.KnownGuilds + ' Nothing on announceNAmes signals failure to apply roles. Set the warning message. + _bot.KnownGuilds(guild.Id).RoleWarning = True + End SyncLock + Return 0 + End If - Throw - End Try If announceNames.Count <> 0 Then ' Send out announcement message Await BirthdayAnnounceAsync(announce, announceping, channel, announceNames) End If - Return announceNames.Count End Function + ''' + ''' Checks if the bot may be allowed to alter roles. + ''' + Private Function BirthdayHasGoodRolePermissions(guild As SocketGuild, role As SocketRole) As Boolean + If Not guild.CurrentUser.GuildPermissions.ManageRoles Then + ' Bot user cannot manage roles + Return False + End If + + ' Check potential role order conflict + If role.Position >= guild.CurrentUser.Hierarchy Then + ' Target role is at or above bot's highest role. + Return False + End If + + Return True + End Function + ''' ''' Gets all known users from the given guild and returns a list including only those who are ''' currently experiencing a birthday in the respective time zone. @@ -187,21 +214,37 @@ Class BackgroundWorker ''' Sets the birthday role to all applicable users. Unsets it from all others who may have it. ''' ''' A list of users who had the birthday role applied. Use for the announcement message. - Private Async Function BirthdayApplyAsync(g As SocketGuild, r As SocketRole, names As HashSet(Of ULong)) As Task(Of IEnumerable(Of SocketGuildUser)) - If Not g.HasAllMembers Then Await g.DownloadUsersAsync() - Dim newBirthdays As New List(Of SocketGuildUser) - For Each user In g.Users - If names.Contains(user.Id) Then - ' User's in the list. Should have the role. Add and make note of if user does not. - If Not user.Roles.Contains(r) Then - Await user.AddRoleAsync(r) - newBirthdays.Add(user) - End If + Private Async Function BirthdayApplyAsync(g As SocketGuild, + r As SocketRole, + names As HashSet(Of ULong)) As Task(Of IEnumerable(Of SocketGuildUser)) + ' Check members currently with the role. Figure out which users to remove it from. + Dim roleRemoves As New List(Of SocketGuildUser) + Dim roleKeeps As New HashSet(Of ULong) + Dim q = 0 + For Each member In r.Members + If Not names.Contains(member.Id) Then + roleRemoves.Add(member) Else - ' User's not in the list. Should remove the role. - If user.Roles.Contains(r) Then Await user.RemoveRoleAsync(r) + roleKeeps.Add(member.Id) End If + q += 1 Next + + ' TODO Can we remove during the iteration instead of after? investigate later... + For Each user In roleRemoves + Await user.RemoveRoleAsync(r) + Next + + ' Apply role to members not already having it. Prepare announcement list. + Dim newBirthdays As New List(Of SocketGuildUser) + For Each target In names + Dim member = g.GetUser(target) + If member Is Nothing Then Continue For + If roleKeeps.Contains(member.Id) Then Continue For ' already has role - do nothing + Await member.AddRoleAsync(r) + newBirthdays.Add(member) + Next + Return newBirthdays End Function diff --git a/BirthdayBot/BirthdayBot.vbproj b/BirthdayBot/BirthdayBot.vbproj index ea26e40..e0f2411 100644 --- a/BirthdayBot/BirthdayBot.vbproj +++ b/BirthdayBot/BirthdayBot.vbproj @@ -4,8 +4,8 @@ Exe BirthdayBot netcoreapp2.0 - 1.0.0 - 1.0.0.0 + 1.0.4 + 1.0.4.0 Noiiko Discord bot for birthday reminders.