Rock paper Scissors with bot command Elias Cueto #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pip install --upgrade certifi '' '
Reviewer's GuideAdds a new Discord Cog that registers a guild-scoped RPS slash command; the command handler builds an embed, logs its invocation, and attempts to react with a predefined set of emojis. Sequence Diagram for /RPS Command InteractionsequenceDiagram
actor User
participant DiscordAPI as Discord API
participant EliasCog
User->>DiscordAPI: Executes /RPS command
DiscordAPI->>EliasCog: Interaction (invokes ping method)
EliasCog->>EliasCog: Log "Choose your reaction"
EliasCog->>EliasCog: Create Embed (title: "Ping", description: "Choose your reaction")
EliasCog->>DiscordAPI: interaction.response.send_message(embed)
DiscordAPI-->>User: Displays message with embed
loop For each emoji in status_emojis.keys
EliasCog->>DiscordAPI: Attempt to add reaction (emoji) to the sent message
end
Class Diagram for the new EliasCogclassDiagram
class Cog {
<<Abstract>>
# In discord.ext.commands
}
class EliasCog {
+bot: commands.Bot
+logger: logging.Logger
+status_emojis: dict
+__init__(bot: commands.Bot)
+ping(interaction: discord.Interaction, arg1) async void
}
EliasCog --|> Cog
EliasCog ..> discord.Interaction : uses
EliasCog ..> discord.Embed : creates
EliasCog ..> logging.Logger : uses
EliasCog ..> commands.Bot : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @EliasCueto45 - I've reviewed your changes - here's some feedback:
- You’re calling await message.add_reaction(...) before you even send or define message—send the embed first, capture the returned Message object, then add reactions to it.
- The status_emojis dict has an empty string for Paper, the slash handler is named ping with an unused arg1, and the embed title reads “Ping” instead of “RPS”—replace the placeholder emoji, rename the method/embed to match RPS, and remove or use arg1.
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ) | ||
| self.status_emojis = { | ||
| "🪨": "Rock", | ||
| "": "Paper", |
There was a problem hiding this comment.
issue (bug_risk): Empty string emoji key for Paper
Replace the empty string key with an appropriate emoji (e.g., 📄) or add logic to handle missing keys.
Comment on lines
24
to
25
| @app_commands.command(name="RPS", description="Plays Rock Paper Scissors with the bot!", ) | ||
| async def ping(self, interaction: discord.Interaction, arg1): |
There was a problem hiding this comment.
suggestion: Command name and handler name mismatched
Please rename the handler and embed title to consistently reference Rock Paper Scissors.
| } | ||
| @app_commands.guilds(discord.Object(id=settings.DEBUG_GUILD_ID)) | ||
| @app_commands.command(name="RPS", description="Plays Rock Paper Scissors with the bot!", ) | ||
| async def ping(self, interaction: discord.Interaction, arg1): |
There was a problem hiding this comment.
suggestion: Unused argument arg1
Remove arg1 if it's unnecessary, or rename it if it will be used later.
Comment on lines
26
to
27
| for emoji in self.status_emojis.keys(): | ||
| await message.add_reaction(emoji) |
There was a problem hiding this comment.
issue (bug_risk): Reacting before message is defined
message must be defined (sent or fetched) before you can add reactions to it. Move the reaction loop after the message is created.
| embed = discord.Embed( | ||
| title="Ping", | ||
| description=message, | ||
| color=colors.PING, |
There was a problem hiding this comment.
suggestion: Using PING color for RPS embed
Consider adding a specific RPS color to config_colors for clarity instead of reusing colors.PING.
Suggested implementation:
embed = discord.Embed(
title="Ping",
description=message,
color=colors.RPS,
)You must also add a new color constant for RPS in your colors or config_colors module, for example:
RPS = 0x3498db # Choose an appropriate hex color codeand ensure it is imported where this file uses colors.
| from config import settings | ||
|
|
||
|
|
||
| class EliasCog(commands.Cog): |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the command into a standalone function without a Cog, custom logger, or unused mappings to simplify the implementation.
You can simplify this to a single `@app_commands.command` (no Cog, no custom logger, no unused mapping) while preserving all behavior:
```python
# commands/rps.py
import discord
from discord import app_commands
from discord.ext import commands
from config import settings
from frontend.config_colors import PING as PING_COLOR
@app_commands.guilds(discord.Object(settings.DEBUG_GUILD_ID))
@app_commands.command(name="rps", description="Plays Rock Paper Scissors with the bot")
async def rps(interaction: discord.Interaction):
embed = discord.Embed(
title="Rock Paper Scissors",
description="Choose your reaction",
color=PING_COLOR,
)
await interaction.response.send_message(embed=embed)
# Grab the sent message and add reactions
message = await interaction.original_response()
for emoji in ("🪨", "📄", "✂️", "⭐", "❌"):
await message.add_reaction(emoji)Steps to apply:
- Drop the
EliasCogclass andsetupfunction entirely. - Create a new module (
commands/rps.py) with the above code. - Ensure your bot’s startup loads
commands/rps.py(e.g. viabot.tree.copy_global_to(guild=...)or your existing command loader). - Remove the unused
status_emojisdict and custom logger import.
This keeps the slash-command, the embed, and the reactions, with far less indirection.
| async def ping(self, interaction: discord.Interaction, arg1): | ||
| for emoji in self.status_emojis.keys(): | ||
| await message.add_reaction(emoji) | ||
| message = f"Choose your reaction" |
There was a problem hiding this comment.
suggestion (code-quality): Replace f-string with no interpolated values with string (remove-redundant-fstring)
Suggested change
| message = f"Choose your reaction" | |
| message = "Choose your reaction" |
EliasCueto45
changed the title
EliasCueto45
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
first draft ups
Summary by Sourcery
New Features: