Conversation
TannPat
commented
Dec 3, 2025
- add database model and CRUD methods
- add commands for adding, getting, and deleting joinphrases
- add logic to send joinphrases on user join
- implement conditions of minimum time and messages since last JP per user per room
- add otherHandler to track messages for miscellaneous use
PartMan7
left a comment
There was a problem hiding this comment.
Hi there's a couple things I've left comments on
Most of the stuff is done though, thanks! Tag me if you'd like me to implement some of the comments I've mentioned
src/database/joinphrases.ts
Outdated
| }, | ||
| userId: { | ||
| type: String, | ||
| default: toId(username), |
There was a problem hiding this comment.
The username here is the bot's username; that's probably not the default we want to go with - ideally this should be a function that takes the given username
src/ps/commands/joinphrases.ts
Outdated
| help: 'Joinphrases module!', | ||
| perms: ['room', 'driver'], | ||
| syntax: 'CMD', | ||
| aliases: ['joinphrase'], |
There was a problem hiding this comment.
aliases and name should be swapped here - the name is the full name of the command, while aliases are abbreviations for those
src/ps/commands/joinphrases.ts
Outdated
| aliases: ['joinphrase'], | ||
| categories: ['utility'], | ||
| extendedAliases: { | ||
| addjp: ['jp', 'new'], |
There was a problem hiding this comment.
Could you also add ajp, djp, and ejp? Those are currently the most commonly used ones
src/ps/commands/joinphrases.ts
Outdated
| const args: string[] = arg.split(',').map(s => s.trim()); | ||
| const username = args[0]; | ||
| const phrase = args[1]; |
There was a problem hiding this comment.
This is broken with commas (phrases with commas will only have the first bit)
| const args: string[] = arg.split(',').map(s => s.trim()); | |
| const username = args[0]; | |
| const phrase = args[1]; | |
| const [username, phrase] = arg.lazySplit(/\s*,\s*/, 1); |
See https://github.com/PartMan7/PartBot/blob/main/src/globals/prototypes.ts#L58 for reference
src/ps/commands/joinphrases.ts
Outdated
| } catch (e: unknown) { | ||
| message.reply(`${username} already has a joinphrase in ${message.target.title}...` as ToTranslate); | ||
| } |
There was a problem hiding this comment.
You don't need to catch explicitly - throwing an error is the preferred approach, since that automatically handles private replies and whatnot
src/ps/handlers/joins.ts
Outdated
| messageCount: number; // messages since last jp | ||
| lastTime: number; // timestamp of last jp | ||
| } | ||
| const jpStateMap: Partial<Record<string, jpState>> = {}; |
There was a problem hiding this comment.
This should be in @/cache instead of here
src/ps/handlers/joins.ts
Outdated
| export function otherHandler(message: PSMessage) { | ||
| if (message.isIntro) return; | ||
| if (!message.author || !message.author.userid || !message.target || message.author.id === message.parent.status.userid) return; | ||
| if (message.content.startsWith('|')) return; | ||
| const roomId = message.target.id; | ||
|
|
||
| for (const key in jpStateMap) { | ||
| if (!jpStateMap[key]) continue; | ||
|
|
||
| const rId = key.split('-')[1]; | ||
| if (rId !== roomId) continue; | ||
|
|
||
| jpStateMap[key]!.messageCount++; | ||
| } // increment message count for each joinphrase in the room | ||
| } | ||
|
|
There was a problem hiding this comment.
This should be in its own file; this file is about join/leave/rename events
src/ps/handlers/joins.ts
Outdated
| state = { messageCount: minimumMessages, lastTime: 0 }; | ||
| jpStateMap[key] = state; | ||
| } | ||
| const now = Date.now() / 1000; |
There was a problem hiding this comment.
Try to use milliseconds instead of seconds wherever possible
src/ps/handlers/joins.ts
Outdated
| for (const key in jpStateMap) { | ||
| if (!jpStateMap[key]) continue; | ||
|
|
||
| const rId = key.split('-')[1]; | ||
| if (rId !== roomId) continue; | ||
|
|
||
| jpStateMap[key]!.messageCount++; | ||
| } // increment message count for each joinphrase in the room |
There was a problem hiding this comment.
Performance wise this is a bit concerning; if there's a couple hundred JPs total, you're looping over every single one of them on every message
Instead, the state map should be grouped by rooms instead so you don't iterate over JPs that aren't for the room
src/ps/handlers/joins.ts
Outdated
| // (Kinda creepy name for the feature, but it CAN be used in creepy ways so make sure it's regulated!) | ||
|
|
||
| const userId = toId(user), | ||
| roomId = toId(room); |
There was a problem hiding this comment.
Why's room been cast to roomId here?
- add database model and CRUD methods - add commands for adding, getting, and deleting joinphrases - add logic to send joinphrases on user join
- add conditions of minimum time and messages since last JP per user per room - add otherHandler to track messages for miscellaneous use
ff79d8e to
9b10ad0
Compare
|
Alright, this looks good now. Thanks! |