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

@@ -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)
+        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
-                    ' Failed to apply role. Set the warning.
+                ' 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
 
+    ''' <summary>
+    ''' Checks if the bot may be allowed to alter roles.
+    ''' </summary>
+    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
+
     ''' <summary>
     ''' 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.
     ''' </summary>
     ''' <returns>A list of users who had the birthday role applied. Use for the announcement message.</returns>
-    Private Async Function BirthdayApplyAsync(g As SocketGuild, r As SocketRole, names As HashSet(Of ULong)) As Task(Of IEnumerable(Of SocketGuildUser))
+    Private Async Function BirthdayApplyAsync(g As SocketGuild,
-        If Not g.HasAllMembers Then Await g.DownloadUsersAsync()
+                                              r As SocketRole,
-        Dim newBirthdays As New List(Of SocketGuildUser)
+                                              names As HashSet(Of ULong)) As Task(Of IEnumerable(Of SocketGuildUser))
-        For Each user In g.Users
+        ' Check members currently with the role. Figure out which users to remove it from.
-            If names.Contains(user.Id) Then
+        Dim roleRemoves As New List(Of SocketGuildUser)
-                ' User's in the list. Should have the role. Add and make note of if user does not.
+        Dim roleKeeps As New HashSet(Of ULong)
-                If Not user.Roles.Contains(r) Then
+        Dim q = 0
-                    Await user.AddRoleAsync(r)
+        For Each member In r.Members
-                    newBirthdays.Add(user)
+            If Not names.Contains(member.Id) Then
-                End If
+                roleRemoves.Add(member)
             Else
-                ' User's not in the list. Should remove the role.
+                roleKeeps.Add(member.Id)
-                If user.Roles.Contains(r) Then Await user.RemoveRoleAsync(r)
             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
 

BirthdayBot/BirthdayBot.vbproj

@@ -4,8 +4,8 @@
     <OutputType>Exe</OutputType>
     <RootNamespace>BirthdayBot</RootNamespace>
     <TargetFramework>netcoreapp2.0</TargetFramework>
-    <Version>1.0.0</Version>
+    <Version>1.0.4</Version>
-    <AssemblyVersion>1.0.0.0</AssemblyVersion>
+    <AssemblyVersion>1.0.4.0</AssemblyVersion>
     <Authors>Noiiko</Authors>
     <Company />
     <Description>Discord bot for birthday reminders.</Description>