Restore "most users downloaded" checks

Workaround to deal with continued instances of inconsistent Discord.Net user downloading behavior

BackgroundServices/AutoUserDownload.cs

@@ -5,8 +5,8 @@ namespace BirthdayBot.BackgroundServices;
 /// <summary>
 /// Proactively fills the user cache for guilds in which any birthday data already exists.
 /// </summary>
-class SelectiveAutoUserDownload : BackgroundService {
+class AutoUserDownload : BackgroundService {
-    public SelectiveAutoUserDownload(ShardInstance instance) : base(instance) { }
+    public AutoUserDownload(ShardInstance instance) : base(instance) { }
 
     public override async Task OnTick(int tickCount, CancellationToken token) {
         foreach (var guild in ShardInstance.DiscordClient.Guilds) {

BackgroundServices/BirthdayRoleUpdate.cs

@@ -40,7 +40,7 @@ class BirthdayRoleUpdate : BackgroundService {
     /// </summary>
     private static async Task ProcessGuildAsync(SocketGuild guild) {
         // Load guild information - stop if local cache is unavailable.
-        if (!guild.HasAllMembers) return;
+        if (!Common.HasMostMembersDownloaded(guild)) return;
         var gc = await GuildConfiguration.LoadAsync(guild.Id, true).ConfigureAwait(false);
         if (gc == null) return;
 

BackgroundServices/ShardBackgroundWorker.cs

@@ -1,97 +1,76 @@
-using System;
+namespace BirthdayBot.BackgroundServices;
-using System.Collections.Generic;
-using System.Threading;
-using System.Threading.Tasks;
 
-namespace BirthdayBot.BackgroundServices
+/// <summary>
-{
+/// Handles the execution of periodic background tasks specific to each shard.
+/// </summary>
+class ShardBackgroundWorker : IDisposable {
     /// <summary>
-    /// Handles the execution of periodic background tasks specific to each shard.
+    /// The interval, in seconds, in which background tasks are attempted to be run within a shard.
     /// </summary>
-    class ShardBackgroundWorker : IDisposable
+    public const int Interval = 40;
-    {
-        /// <summary>
-        /// The interval, in seconds, in which background tasks are attempted to be run within a shard.
-        /// </summary>
-        public const int Interval = 40;
 
-        private readonly Task _workerTask;
+    private readonly Task _workerTask;
-        private readonly CancellationTokenSource _workerCanceller;
+    private readonly CancellationTokenSource _workerCanceller;
-        private readonly List<BackgroundService> _workers;
+    private readonly List<BackgroundService> _workers;
-        private int _tickCount = -1;
+    private int _tickCount = -1;
 
-        private ShardInstance Instance { get; }
+    private ShardInstance Instance { get; }
 
-        public BirthdayRoleUpdate BirthdayUpdater { get; }
+    public DateTimeOffset LastBackgroundRun { get; private set; }
-        public SelectiveAutoUserDownload UserDownloader { get; }
+    public string? CurrentExecutingService { get; private set; }
-        public DateTimeOffset LastBackgroundRun { get; private set; }
-        public string? CurrentExecutingService { get; private set; }
 
-        public ShardBackgroundWorker(ShardInstance instance)
+    public ShardBackgroundWorker(ShardInstance instance) {
+        Instance = instance;
+        _workerCanceller = new CancellationTokenSource();
+
+        _workers = new List<BackgroundService>()
         {
-            Instance = instance;
+                {new AutoUserDownload(instance)},
-            _workerCanceller = new CancellationTokenSource();
+                {new BirthdayRoleUpdate(instance)},
-
-            BirthdayUpdater = new BirthdayRoleUpdate(instance);
-            UserDownloader = new SelectiveAutoUserDownload(instance);
-            _workers = new List<BackgroundService>()
-            {
-                {UserDownloader},
-                {BirthdayUpdater},
                 {new DataRetention(instance)},
                 {new ExternalStatisticsReporting(instance)}
             };
 
-            _workerTask = Task.Factory.StartNew(WorkerLoop, _workerCanceller.Token);
+        _workerTask = Task.Factory.StartNew(WorkerLoop, _workerCanceller.Token);
-        }
+    }
 
-        public void Dispose()
+    public void Dispose() {
-        {
+        _workerCanceller.Cancel();
-            _workerCanceller.Cancel();
+        _workerTask.Wait(5000);
-            _workerTask.Wait(5000);
+        if (!_workerTask.IsCompleted)
-            if (!_workerTask.IsCompleted)
+            Instance.Log("Dispose", "Warning: Background worker has not yet stopped. Forcing its disposal.");
-                Instance.Log("Dispose", "Warning: Background worker has not yet stopped. Forcing its disposal.");
+        _workerTask.Dispose();
-            _workerTask.Dispose();
+        _workerCanceller.Dispose();
-            _workerCanceller.Dispose();
+    }
-        }
 
-        /// <summary>
+    /// <summary>
-        /// *The* background task for the shard.
+    /// *The* background task for the shard.
-        /// Executes service tasks and handles errors.
+    /// Executes service tasks and handles errors.
-        /// </summary>
+    /// </summary>
-        private async Task WorkerLoop()
+    private async Task WorkerLoop() {
-        {
+        LastBackgroundRun = DateTimeOffset.UtcNow;
-            LastBackgroundRun = DateTimeOffset.UtcNow;
+        try {
-            try
+            while (!_workerCanceller.IsCancellationRequested) {
-            {
+                await Task.Delay(Interval * 1000, _workerCanceller.Token).ConfigureAwait(false);
-                while (!_workerCanceller.IsCancellationRequested)
-                {
-                    await Task.Delay(Interval * 1000, _workerCanceller.Token).ConfigureAwait(false);
 
-                    // Skip this round of task execution if the client is not connected
+                // Skip this round of task execution if the client is not connected
-                    if (Instance.DiscordClient.ConnectionState != Discord.ConnectionState.Connected) continue;
+                if (Instance.DiscordClient.ConnectionState != Discord.ConnectionState.Connected) continue;
 
-                    // Execute tasks sequentially
+                // Execute tasks sequentially
-                    foreach (var service in _workers)
+                foreach (var service in _workers) {
-                    {
+                    CurrentExecutingService = service.GetType().Name;
-                        CurrentExecutingService = service.GetType().Name;
+                    try {
-                        try
+                        if (_workerCanceller.IsCancellationRequested) break;
-                        {
+                        _tickCount++;
-                            if (_workerCanceller.IsCancellationRequested) break;
+                        await service.OnTick(_tickCount, _workerCanceller.Token).ConfigureAwait(false);
-                            _tickCount++;
+                    } catch (Exception ex) when (ex is not TaskCanceledException) {
-                            await service.OnTick(_tickCount, _workerCanceller.Token).ConfigureAwait(false);
+                        // TODO webhook log
-                        }
+                        Instance.Log(nameof(WorkerLoop), $"{CurrentExecutingService} encountered an exception:\n" + ex.ToString());
-                        catch (Exception ex) when (ex is not TaskCanceledException)
-                        {
-                            // TODO webhook log
-                            Instance.Log(nameof(WorkerLoop), $"{CurrentExecutingService} encountered an exception:\n" + ex.ToString());
-                        }
                     }
-                    CurrentExecutingService = null;
-                    LastBackgroundRun = DateTimeOffset.UtcNow;
                 }
+                CurrentExecutingService = null;
+                LastBackgroundRun = DateTimeOffset.UtcNow;
             }
-            catch (TaskCanceledException) { }
+        } catch (TaskCanceledException) { }
-        }
     }
 }

Common.cs

@@ -31,4 +31,22 @@ static class Common {
         { 1, "Jan" }, { 2, "Feb" }, { 3, "Mar" }, { 4, "Apr" }, { 5, "May" }, { 6, "Jun" },
         { 7, "Jul" }, { 8, "Aug" }, { 9, "Sep" }, { 10, "Oct" }, { 11, "Nov" }, { 12, "Dec" }
     };
+
+    /// <summary>
+    /// An alternative to <see cref="SocketGuild.HasAllMembers"/>.
+    /// Returns true if *most* members have been downloaded.
+    /// Used as a workaround check due to Discord.Net occasionally unable to actually download all members.
+    /// </summary>
+    public static bool HasMostMembersDownloaded(SocketGuild guild) {
+        if (guild.HasAllMembers) return true;
+        if (guild.MemberCount > 30) {
+            // For guilds of size over 30, require 85% or more of the members to be known
+            // (26/30, 42/50, 255/300, etc)
+            int threshold = (int)(guild.MemberCount * 0.85);
+            return guild.DownloadedMemberCount >= threshold;
+        } else {
+            // For smaller guilds, fail if two or more members are missing
+            return guild.MemberCount - guild.DownloadedMemberCount <= 2;
+        }
+    }
 }

UserInterface/CommandsCommon.cs

@@ -82,11 +82,11 @@ internal abstract class CommandsCommon {
     /// </returns>
     /// <remarks>
     /// Any updates to the member cache aren't accessible until the event handler finishes execution, meaning proactive downloading
-    /// is necessary, and is handled by <seealso cref="BackgroundServices.SelectiveAutoUserDownload"/>. In situations where
+    /// is necessary, and is handled by <seealso cref="BackgroundServices.AutoUserDownload"/>. In situations where
     /// this approach fails, this is to be called, and the user must be asked to attempt the command again if this returns false.
     /// </remarks>
     protected static async Task<bool> HasMemberCacheAsync(SocketGuild guild) {
-        if (guild.HasAllMembers) return true;
+        if (Common.HasMostMembersDownloaded(guild)) return true;
         // Event handling thread hangs if awaited normally or used with Task.Run
         await Task.Factory.StartNew(guild.DownloadUsersAsync).ConfigureAwait(false);
         return false;

UserInterface/ManagerCommands.cs

@@ -387,7 +387,7 @@ internal class ManagerCommands : CommandsCommon {
         var conf = await GuildConfiguration.LoadAsync(guild.Id, true).ConfigureAwait(false);
 
         result.AppendLine($"Server ID: {guild.Id} | Bot shard ID: {instance.ShardId:00}");
-        bool hasMembers = guild.HasAllMembers;
+        bool hasMembers = Common.HasMostMembersDownloaded(guild);
         result.Append(DoTestFor("Bot has obtained the user list", () => hasMembers));
         result.AppendLine($" - Has {guild.DownloadedMemberCount} of {guild.MemberCount} members.");
         int bdayCount = -1;