diff --git a/BackgroundServices/SelectiveAutoUserDownload.cs b/BackgroundServices/AutoUserDownload.cs similarity index 92% rename from BackgroundServices/SelectiveAutoUserDownload.cs rename to BackgroundServices/AutoUserDownload.cs index a042973..ecb90c3 100644 --- a/BackgroundServices/SelectiveAutoUserDownload.cs +++ b/BackgroundServices/AutoUserDownload.cs @@ -5,8 +5,8 @@ namespace BirthdayBot.BackgroundServices; /// /// Proactively fills the user cache for guilds in which any birthday data already exists. /// -class SelectiveAutoUserDownload : BackgroundService { - public SelectiveAutoUserDownload(ShardInstance instance) : base(instance) { } +class AutoUserDownload : BackgroundService { + public AutoUserDownload(ShardInstance instance) : base(instance) { } public override async Task OnTick(int tickCount, CancellationToken token) { foreach (var guild in ShardInstance.DiscordClient.Guilds) { diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index 51e42c5..03622d0 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -40,7 +40,7 @@ class BirthdayRoleUpdate : BackgroundService { /// 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; diff --git a/BackgroundServices/ShardBackgroundWorker.cs b/BackgroundServices/ShardBackgroundWorker.cs index 6c99e15..8ebbdfe 100644 --- a/BackgroundServices/ShardBackgroundWorker.cs +++ b/BackgroundServices/ShardBackgroundWorker.cs @@ -1,97 +1,76 @@ -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; +namespace BirthdayBot.BackgroundServices; -namespace BirthdayBot.BackgroundServices -{ +/// +/// Handles the execution of periodic background tasks specific to each shard. +/// +class ShardBackgroundWorker : IDisposable { /// - /// 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. /// - class ShardBackgroundWorker : IDisposable - { - /// - /// The interval, in seconds, in which background tasks are attempted to be run within a shard. - /// - public const int Interval = 40; + public const int Interval = 40; - private readonly Task _workerTask; - private readonly CancellationTokenSource _workerCanceller; - private readonly List _workers; - private int _tickCount = -1; + private readonly Task _workerTask; + private readonly CancellationTokenSource _workerCanceller; + private readonly List _workers; + private int _tickCount = -1; - private ShardInstance Instance { get; } + private ShardInstance Instance { get; } - public BirthdayRoleUpdate BirthdayUpdater { get; } - public SelectiveAutoUserDownload UserDownloader { get; } - public DateTimeOffset LastBackgroundRun { get; private set; } - 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() { - Instance = instance; - _workerCanceller = new CancellationTokenSource(); - - BirthdayUpdater = new BirthdayRoleUpdate(instance); - UserDownloader = new SelectiveAutoUserDownload(instance); - _workers = new List() - { - {UserDownloader}, - {BirthdayUpdater}, + {new AutoUserDownload(instance)}, + {new BirthdayRoleUpdate(instance)}, {new DataRetention(instance)}, {new ExternalStatisticsReporting(instance)} }; - _workerTask = Task.Factory.StartNew(WorkerLoop, _workerCanceller.Token); - } + _workerTask = Task.Factory.StartNew(WorkerLoop, _workerCanceller.Token); + } - public void Dispose() - { - _workerCanceller.Cancel(); - _workerTask.Wait(5000); - if (!_workerTask.IsCompleted) - Instance.Log("Dispose", "Warning: Background worker has not yet stopped. Forcing its disposal."); - _workerTask.Dispose(); - _workerCanceller.Dispose(); - } + public void Dispose() { + _workerCanceller.Cancel(); + _workerTask.Wait(5000); + if (!_workerTask.IsCompleted) + Instance.Log("Dispose", "Warning: Background worker has not yet stopped. Forcing its disposal."); + _workerTask.Dispose(); + _workerCanceller.Dispose(); + } - /// - /// *The* background task for the shard. - /// Executes service tasks and handles errors. - /// - private async Task WorkerLoop() - { - LastBackgroundRun = DateTimeOffset.UtcNow; - try - { - while (!_workerCanceller.IsCancellationRequested) - { - await Task.Delay(Interval * 1000, _workerCanceller.Token).ConfigureAwait(false); + /// + /// *The* background task for the shard. + /// Executes service tasks and handles errors. + /// + private async Task WorkerLoop() { + LastBackgroundRun = DateTimeOffset.UtcNow; + try { + while (!_workerCanceller.IsCancellationRequested) { + await Task.Delay(Interval * 1000, _workerCanceller.Token).ConfigureAwait(false); - // Skip this round of task execution if the client is not connected - if (Instance.DiscordClient.ConnectionState != Discord.ConnectionState.Connected) continue; + // Skip this round of task execution if the client is not connected + if (Instance.DiscordClient.ConnectionState != Discord.ConnectionState.Connected) continue; - // Execute tasks sequentially - foreach (var service in _workers) - { - CurrentExecutingService = service.GetType().Name; - try - { - if (_workerCanceller.IsCancellationRequested) break; - _tickCount++; - await service.OnTick(_tickCount, _workerCanceller.Token).ConfigureAwait(false); - } - catch (Exception ex) when (ex is not TaskCanceledException) - { - // TODO webhook log - Instance.Log(nameof(WorkerLoop), $"{CurrentExecutingService} encountered an exception:\n" + ex.ToString()); - } + // Execute tasks sequentially + foreach (var service in _workers) { + CurrentExecutingService = service.GetType().Name; + try { + if (_workerCanceller.IsCancellationRequested) break; + _tickCount++; + await service.OnTick(_tickCount, _workerCanceller.Token).ConfigureAwait(false); + } 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) { } } } diff --git a/Common.cs b/Common.cs index a9a741a..55c85b4 100644 --- a/Common.cs +++ b/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" } }; + + /// + /// An alternative to . + /// Returns true if *most* members have been downloaded. + /// Used as a workaround check due to Discord.Net occasionally unable to actually download all members. + /// + 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; + } + } } diff --git a/UserInterface/CommandsCommon.cs b/UserInterface/CommandsCommon.cs index 93ab296..9231096 100644 --- a/UserInterface/CommandsCommon.cs +++ b/UserInterface/CommandsCommon.cs @@ -82,11 +82,11 @@ internal abstract class CommandsCommon { /// /// /// Any updates to the member cache aren't accessible until the event handler finishes execution, meaning proactive downloading - /// is necessary, and is handled by . In situations where + /// is necessary, and is handled by . 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. /// protected static async Task 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; diff --git a/UserInterface/ManagerCommands.cs b/UserInterface/ManagerCommands.cs index ac8e8b6..0d79275 100644 --- a/UserInterface/ManagerCommands.cs +++ b/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;