✨feat: replace mobygenerator with local bottools.GetRandomName#2306
✨feat: replace mobygenerator with local bottools.GetRandomName#2306
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the external moby/namesgenerator dependency with a local bottools.GetRandomName helper for generating docker-style names.
Changes:
- Removed
github.com/moby/moby/pkg/namesgeneratorusage and dependency from the module. - Added
src/bottools/random_name.goimplementingbottools.GetRandomName. - Updated call sites in tracking and boost flows to use the new local name generator.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/track/track_slashcmd.go | Uses bottools.GetRandomName for default token tracking session names. |
| src/bottools/random_name.go | Adds a local random-name generator implementation. |
| src/boost/roles.go | Switches role/team-name generation fallback to bottools.GetRandomName. |
| src/boost/contract.go | Switches contract hash candidate generation to bottools.GetRandomName. |
| src/boost/boost_reactions.go | Switches placeholder farmer-name generation to bottools.GetRandomName. |
| go.mod | Removes the github.com/moby/moby dependency. |
| go.sum | Removes github.com/moby/moby checksums. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func GetRandomName(_ int) string { | ||
| left := randomNameLeft[rand.IntN(len(randomNameLeft))] | ||
| right := randomNameRight[rand.IntN(len(randomNameRight))] |
There was a problem hiding this comment.
GetRandomName currently ignores its input parameter (it’s named _ int). Since this is an exported helper, either remove the parameter and update call sites, or document that it’s kept only for compatibility; ideally, use it (e.g., as a deterministic seed) so callers/tests can reproduce names when needed.
| func GetRandomName(_ int) string { | |
| left := randomNameLeft[rand.IntN(len(randomNameLeft))] | |
| right := randomNameRight[rand.IntN(len(randomNameRight))] | |
| // If seed is non-zero, the returned name is deterministic for that seed. | |
| // If seed is zero, a non-deterministic package-level generator is used. | |
| func GetRandomName(seed int) string { | |
| if seed == 0 { | |
| left := randomNameLeft[rand.IntN(len(randomNameLeft))] | |
| right := randomNameRight[rand.IntN(len(randomNameRight))] | |
| return fmt.Sprintf("%s_%s", left, right) | |
| } | |
| r := rand.New(rand.NewPCG(uint64(seed), uint64(seed)^0x9e3779b97f4a7c15)) | |
| left := randomNameLeft[r.IntN(len(randomNameLeft))] | |
| right := randomNameRight[r.IntN(len(randomNameRight))] |
| var randomNameLeft = []string{ | ||
| "brisk", "calm", "clever", "cosmic", "crisp", "daring", "gentle", "lucky", "mellow", "nimble", | ||
| "quiet", "rapid", "savvy", "steady", "swift", "vivid", "witty", "bold", "bright", "chill", | ||
| } | ||
|
|
||
| var randomNameRight = []string{ | ||
| "acorn", "anchor", "aster", "beacon", "bison", "comet", "drifter", "falcon", "harbor", "meadow", | ||
| "nebula", "otter", "ranger", "rocket", "sailor", "sprout", "thunder", "valley", "voyager", "zephyr", | ||
| } |
There was a problem hiding this comment.
The adjective/noun lists yield only 400 possible combinations, which makes collisions fairly likely in flows that assume uniqueness (e.g., placeholder farmers added via reactions, random token tracker names). Consider expanding the wordlists and/or adding a short random suffix inside GetRandomName to reduce collision probability.
| if creatorOfContract(s, contract, r.UserID) { | ||
| for i := len(contract.Order); i < contract.CoopSize; i++ { | ||
| _, err := AddFarmerToContract(s, contract, r.GuildID, r.ChannelID, namesgenerator.GetRandomName(0), contract.BoostOrder, true) | ||
| _, err := AddFarmerToContract(s, contract, r.GuildID, r.ChannelID, bottools.GetRandomName(0), contract.BoostOrder, true) | ||
| if err != nil { |
There was a problem hiding this comment.
The 🐿️ autofill loop increments i regardless of whether AddFarmerToContract actually added a new booster. With random-name IDs, collisions can cause AddFarmerToContract to no-op (existing booster) and the loop will finish with fewer than contract.CoopSize members. Generate IDs guaranteed-unique for the contract (e.g., retry until contract.Boosters[cand]==nil, or append a short random suffix/xid).
| } else if trackingName == "" { | ||
|
|
||
| trackingName = fmt.Sprintln(namesgenerator.GetRandomName(0)) | ||
| trackingName = bottools.GetRandomName(0) |
There was a problem hiding this comment.
When coop-id is omitted and trackingName falls back to a random value, there’s no check that the generated name is unused for this user. If it collides with an existing tracker name, tokenTracking will delete the prior tracker message and reset its data. Consider generating until unused (check Tokens[userID].Coop) or adding a short unique suffix (e.g., xid) to avoid accidental overwrites.
| trackingName = bottools.GetRandomName(0) | |
| trackingName = fmt.Sprintf("%s-%s", bottools.GetRandomName(0), xid.New().String()[:6]) |
| // GetRandomName returns a docker-style random name. | ||
| func GetRandomName(_ int) string { | ||
| left := randomNameLeft[rand.IntN(len(randomNameLeft))] | ||
| right := randomNameRight[rand.IntN(len(randomNameRight))] | ||
| return fmt.Sprintf("%s_%s", left, right) | ||
| } |
There was a problem hiding this comment.
This introduces a new random-name generator that is now used in several user-visible/code-path identifiers. Since the repo runs go test -race ./... in CI and src/bottools already has tests, please add unit coverage for GetRandomName (at least: format invariants, non-empty output, and—if a seed parameter is supported—deterministic output for a fixed seed) to prevent accidental regressions.
No description provided.