-
-
Notifications
You must be signed in to change notification settings - Fork 311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modify Custom Vote #848
Modify Custom Vote #848
Conversation
WalkthroughThe changes in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
Content.Server/Voting/VoteCommands.cs (2)
Line range hint
32-39
: OwO notices your argument validation! nuzzles the codeThe validation wogic looks good but could use some extra safety! paws at the code Maybe we should add some input sanitation for the votekick target? >w<
if (args.Length != 3 && args[0] == StandardVoteType.Votekick.ToString()) { shell.WriteError(Loc.GetString("shell-wrong-arguments-number-need-specific", ("properAmount", 3), ("currentAmount", args.Length))); return; } + // Add validation for votekick target + if (args[0] == StandardVoteType.Votekick.ToString() && string.IsNullOrWhiteSpace(args[1])) + { + shell.WriteError(Loc.GetString("Invalid votekick target")); + return; + }
Line range hint
112-119
: *pounces on your logging* Rawr x3The logging is getting better! nuzzles the code But we could make it even more detailed! >w<
if (shell.Player != null) - _adminLogger.Add(LogType.Vote, LogImpact.Medium, $"{shell.Player} initiated a custom vote: {options.Title} - {string.Join("; ", options.Options.Select(x => x.text))}"); + _adminLogger.Add(LogType.Vote, LogImpact.Medium, $"{shell.Player} initiated a custom vote: {options.Title} - {string.Join("; ", options.Options.Select(x => x.text))} with duration {options.Duration.TotalSeconds}s");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Content.Server/Voting/VoteCommands.cs
(1 hunks)
🔇 Additional comments (2)
Content.Server/Voting/VoteCommands.cs (2)
82-82
: *notices your big number* What's this? owo
The MaxArgCount of 31 seems weawwy high! boops the code This could make the vote UI messy and might be abused to spam chat! Maybe we should:
- Add a check for duplicate options
- Validate option lengths
- Consider a more reasonable limit? :3
Also applies to: 90-93
90-93
:
*notices potential security risk* owo what's this?
With the lowered minimum args (>﹏<) we should make sure players can't create misleading votes! worried paw gestures
Consider adding validation for:
- Minimum option length
- Restricted characters
- Rate limiting for non-admin votes
Also applies to: 112-119
About the PR
Why / Balance
why not
Summary by CodeRabbit
New Features
Bug Fixes
Votekick
command to ensure exactly three arguments are provided.Documentation