Skip to content

🔧 fix(contract): improve team name generation and state reset logic#1909

Merged
mkmccarty merged 1 commit intomainfrom
mm-branch-1
Dec 6, 2025
Merged

🔧 fix(contract): improve team name generation and state reset logic#1909
mkmccarty merged 1 commit intomainfrom
mm-branch-1

Conversation

@mkmccarty
Copy link
Owner

No description provided.

Copilot AI review requested due to automatic review settings December 6, 2025 04:52
@mkmccarty mkmccarty merged commit ef8f80f into main Dec 6, 2025
8 checks passed
@mkmccarty mkmccarty deleted the mm-branch-1 branch December 6, 2025 04:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the team name generation logic and fixes state reset behavior in contract settings. The changes address two main areas: a more robust fallback mechanism for team name generation when all preferred names are taken, and fixing the state cycling logic to properly reset to the first alternative.

  • Introduces a lastChance flag to prevent infinite loops and adds proper tryCount reset when falling back to secondary name lists
  • Fixes state reset logic to cycle back to the first alternative (alts[0]) instead of staying on the current one (alts[altIdx])
  • Adds proper prefix handling ("Team ") when falling back to generic random names

@@ -491,11 +491,8 @@ func getContractRole(s *discordgo.Session, guildID string, contract *Contract) e
unusedRoleNames[i], unusedRoleNames[j] = unusedRoleNames[j], unusedRoleNames[i]
})

Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

Potential panic risk: When unusedRoleNames is empty (which can happen if all roleNames are already in existingRoles), accessing unusedRoleNames[tryCount] at line 496 will cause an index out of range panic.

The code should check if unusedRoleNames is empty before entering the loop, or handle the empty case within the loop. Consider adding:

if len(unusedRoleNames) == 0 {
    // Jump directly to fallback logic
    unusedRoleNames = randomThingNames
    rand.Shuffle(len(unusedRoleNames), func(i, j int) {
        unusedRoleNames[i], unusedRoleNames[j] = unusedRoleNames[j], unusedRoleNames[i]
    })
    prefix = "Team "
}
Suggested change
// If no unused role names, jump to fallback logic
if len(unusedRoleNames) == 0 {
unusedRoleNames = randomThingNames
rand.Shuffle(len(unusedRoleNames), func(i, j int) {
unusedRoleNames[i], unusedRoleNames[j] = unusedRoleNames[j], unusedRoleNames[i]
})
prefix = "Team "
}

Copilot uses AI. Check for mistakes.
break
}
name := unusedRoleNames[tryCount]
if !slices.Contains(existingRoles, name) {
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

Logic issue: The check !slices.Contains(existingRoles, name) at line 497 will not work correctly after the reset because prefix is now "Team " but name doesn't include the prefix. After the reset, you're checking if the unprefixed name exists in existingRoles, but you should be checking prefix + name.

Change line 497 to:

if !slices.Contains(existingRoles, prefix+name) {

And update line 499 to store the name without the prefix:

teamName = name

This way line 520 (fmt.Sprintf("%s%s", prefix, teamName)) will correctly combine them.

Suggested change
if !slices.Contains(existingRoles, name) {
if !slices.Contains(existingRoles, prefix+name) {

Copilot uses AI. Check for mistakes.
@mkmccarty
Copy link
Owner Author

@copilot open a new pull request to apply changes based on the comments in this thread

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.

1 participant