feat: add command to ping random members#103
feat: add command to ping random members#103AnandajithS wants to merge 1 commit intoamfoss:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new $random prefix command to ping a random subset of members with either the First Year or Second Year role, wiring it into the bot’s command registry and introducing a rand dependency.
Changes:
- Added
$randomcommand implementation to fetch guild members, filter by year roles, and ping 5 selected members. - Registered the new command in
src/commands/mod.rs. - Added role ID constants for First/Second Year and added
randas a dependency.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ids.rs | Adds role ID constants for First/Second Year filtering. |
| src/commands/random.rs | Implements $random member selection and ping message. |
| src/commands/mod.rs | Registers the new random command in the command list. |
| Cargo.toml | Adds direct rand dependency. |
| Cargo.lock | Records rand addition and multiple dependency updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[poise::command(prefix_command)] | ||
| pub async fn random(ctx: Context<'_>) -> Result<(), Error> { |
There was a problem hiding this comment.
$random pings 5 users and is currently callable by anyone. To prevent abuse/spam, consider restricting it (e.g., owners_only or check = "is_privileged") and/or adding a per-user cooldown.
| let ping_message = selected | ||
| .iter() | ||
| .map(|m| m.user.mention().to_string()) | ||
| .collect::<Vec<_>>() | ||
| .join("\n"); | ||
|
|
||
| ctx.say(format!("Pinging 5 members: {}", ping_message)) | ||
| .await?; |
There was a problem hiding this comment.
The response text always says "Pinging 5 members" even if fewer than 5 eligible members exist (or if selection returns fewer for any reason), which can produce misleading output and an empty message body. Use selected.len() in the message and handle the zero-selected case with a friendly reply.
| .filter(|m| { | ||
| !m.user.bot | ||
| && (m.roles.contains(&RoleId::new(FIRST_YEAR_ROLE_ID)) | ||
| || m.roles.contains(&RoleId::new(SECOND_YEAR_ROLE_ID))) | ||
| }) |
There was a problem hiding this comment.
The role IDs are converted to RoleId inside the filter closure for every member. Hoist let first_year = RoleId::new(...); let second_year = ...; outside the iterator to avoid repeated construction and keep the filter easier to read.
| mailparse = "0.15" | ||
| html2text = "0.12" No newline at end of file | ||
| html2text = "0.12" | ||
| rand = "0.10" No newline at end of file |
There was a problem hiding this comment.
Adding rand = "0.10" introduces a second major rand version in the build (Cargo.lock now contains both rand 0.8.5 and rand 0.10.0). If you don’t need 0.10-specific APIs, prefer aligning to the existing rand 0.8.x to reduce compile time and binary size; otherwise consider documenting why 0.10 is required.
| let guild = ctx.guild_id().ok_or("No guild id")?; | ||
| let members = guild.members(ctx.http(), None, None).await?; | ||
|
|
There was a problem hiding this comment.
This command bulk-fetches the guild member list via the REST API (GuildId::members). Discord restricts this endpoint and it can fail (or return partial data) unless the application has the Guild Members privileged intent enabled; it’s also potentially expensive on larger guilds. Consider either enabling/requesting the needed intent and using a paginated/streaming approach, or refactoring to avoid full member enumeration if possible.
This PR adds a
$randomcommand which randomly selects and pings 5 members who are either First Years or Second YearsChanges
$randomcommand.shufflemethod