Skip to content

fix(team-randomizer): add safety improvements#417

Open
lbzepoqo wants to merge 2 commits into
Team-Silver-Sphere:masterfrom
lbzepoqo:fix/team-randomizer-improvements
Open

fix(team-randomizer): add safety improvements#417
lbzepoqo wants to merge 2 commits into
Team-Silver-Sphere:masterfrom
lbzepoqo:fix/team-randomizer-improvements

Conversation

@lbzepoqo

Copy link
Copy Markdown
Contributor

Summary

Several safety and UX issues in TeamRandomizer that affect production servers:

  • Disabled by default — changed defaultEnabled from true to false. This is a destructive, irreversible-during-round operation; it shouldn't be on without explicit opt-in.
  • Re-entrancy guard — concurrent !randomize calls from multiple admins would run simultaneously and clobber each other. Added a this.randomizing flag with try/finally to ensure it always resets even on error.
  • Error handling per player — the original loop had no try/catch; a single failed switchTeam RCON call would abort mid-shuffle, leaving teams in a partially-randomized state. Now each switch is caught individually, logs the failure, and continues.
  • Player broadcast — players were silently moved to different teams with no explanation. Added an AdminBroadcast before switching starts.
  • Admin feedback — the admin who ran the command got no confirmation. Added a warn on completion with the count of players switched.
  • Shuffle cleanup — removed the unnecessary temporaryValue temp variable; replaced with destructuring swap.

Test plan

  • Run !randomize in admin chat — confirm broadcast fires, teams are shuffled, admin receives completion message
  • Run !randomize twice simultaneously — confirm second call is rejected with "already in progress" message
  • Verify plugin is opt-in by default (not enabled in generated config)

@lbzepoqo lbzepoqo requested a review from werewolfboy13 as a code owner April 15, 2026 16:16

@fantinodavide fantinodavide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lbzepoqo the plugin file itself looks good, but the pre-commit hook didn't run, which is a mandatory step for changes like this one.

@lbzepoqo lbzepoqo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-commit hook steps have now been applied: lint (ESLint + Prettier) reformatted team-randomizer.js, and yarn build-all regenerated README.md and config.json. Sorry for the oversight!

@lbzepoqo lbzepoqo force-pushed the fix/team-randomizer-improvements branch from 872d622 to 8f1c661 Compare April 16, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants