[hitandblow] Migrate to ChannelLimitedBot for #tsgbot-games support#1150
[hitandblow] Migrate to ChannelLimitedBot for #tsgbot-games support#1150
Conversation
Co-authored-by: hakatashi <3126484+hakatashi@users.noreply.github.com>
|
|
えへ～ ありがとう！ レビューさせてもらうにゃ！ |
![tsg-ut-gemini-assistant[bot]](https://avatars.githubusercontent.com/in/1618202?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
このプルリクエストは、hitandblow ボットを ChannelLimitedBot に移行して、#tsgbot-games チャンネルに対応させるものだにゃ。全体的に、以前のボットの移行パターンにうまく従っていて、とても良いリファクタリングだと思うにゃ！コードもきれいになったにゃししっ！
🔍 General Feedback
- ボットを
ChannelLimitedBotに移行することで、チャンネル制限や進行状況の通知が自動的にできるようになるのはすごく便利だにゃ！ - テストファイル (
index.test.ts) のjest.mockの変更も、新しいアーキテクチャに合わせてていい感じだにゃ！ onWakeWordやonMessageEventの実装も、新しいクラスベースの構造に合っていて、ちゃんとエラーハンドリングもしてるのがえらいにゃ！
| }) => { | ||
| const state = new HitAndBlowState(); | ||
| class HitAndBlowBot extends ChannelLimitedBot { | ||
| protected override readonly wakeWordRegex = /^hitandblow( \d+)?$/; |
There was a problem hiding this comment.
🟢 アイコン絵文字がプレースホルダーのままだにゃ！ちゃんとした絵文字に差し替えるのを忘れないようにするんだにゃ！にゃししっ！
| protected override readonly iconEmoji = '1234'; | ||
|
|
||
| private state = new HitAndBlowState(); | ||
|
|
There was a problem hiding this comment.
🟢 うなの考えでは、このコメントはもういらないんじゃないかと思うにゃ！だって、うながゲームを開始するときにちゃんとchannelを設定してるんだにゃ！だから、nullになる心配はないんだにゃ！
| channel: this.state.channel, |
| message === null || | ||
| !message.text || | ||
| message.subtype | ||
| ) { |
There was a problem hiding this comment.
🟢 このコメントもいらないと思うにゃ！うなの独り言はコードに残さないほうがいいんだにゃ！
| ) { | |
| hits.size | |
| } Hit ${blows.size - hits.size} Blow`, |
There was a problem hiding this comment.
Pull request overview
This pull request migrates the hitandblow bot to use the ChannelLimitedBot base class, enabling it to run in the #tsgbot-games channel with automatic channel restrictions and progress tracking. The refactoring follows the established patterns from similar migrations (anime #1138, character-quiz #1122, achievement-quiz #1124).
Changes:
- Converted the bot from functional export to class-based architecture extending
ChannelLimitedBot - Implemented channel enforcement with automatic message redirect and cleanup for disallowed channels
- Added progress notifications to
#tsgbot-gamesthat are cleaned up on game completion
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| hitandblow/index.ts | Refactored to HitAndBlowBot class extending ChannelLimitedBot; replaced direct Slack API calls with unified postMessage method; added onWakeWord for game initialization with Deferred-based promise handling; implemented onMessageEvent override for game logic; added deleteProgressMessage calls on game end |
| hitandblow/index.test.ts | Simplified test mocking by removing specific isPlayground mock and using generic slackUtils mock, consistent with ChannelLimitedBot pattern |
| class HitAndBlowBot extends ChannelLimitedBot { | ||
| protected override readonly wakeWordRegex = /^hitandblow( \d+)?$/; | ||
| protected override readonly username = 'Hit & Blow'; | ||
| protected override readonly iconEmoji = '1234'; |
There was a problem hiding this comment.
The iconEmoji should use Slack emoji format with colons. Change '1234' to ':1234:' to match the pattern used by other bots in the codebase (see achievement-quiz/index.ts:111, character-quiz/index.ts:343, wordhero/crossword.ts:107, etc.). Without colons, Slack may not properly render the emoji.
| 答えは \`${this.state.answer | ||
| .map((dig: number) => String(dig)) | ||
| .join('')}\` だよ:cry:`, | ||
| channel: process.env.CHANNEL_SANDBOX as string, |
There was a problem hiding this comment.
Commented-out giveup feature has hardcoded reference to CHANNEL_SANDBOX. If this feature is re-enabled, line 329 should be updated to use this.state.channel instead of process.env.CHANNEL_SANDBOX as string to match the pattern used elsewhere in the code.
| channel: process.env.CHANNEL_SANDBOX as string, | |
| channel: this.state.channel, |
| reply_broadcast: true, | ||
| }); | ||
| postHistory(state.history); | ||
| await this.postHistory(this.state.history); |
There was a problem hiding this comment.
Commented-out giveup feature is missing a call to await this.deleteProgressMessage(this.state.thread) before clearing the state. This should be added (after line 333) to match the pattern used in the timeUp method (line 145) and the win condition (line 311), ensuring progress messages are cleaned up when the game ends.
| await this.postHistory(this.state.history); | |
| await this.postHistory(this.state.history); | |
| await this.deleteProgressMessage(this.state.thread); |
| })().catch((error: unknown) => { | ||
| this.log.error('Failed to start hitandblow game', error); | ||
| const errorText = | ||
| error instanceof Error && error.stack !== undefined | ||
| ? error.stack : String(error); | ||
| this.postMessage({ |
There was a problem hiding this comment.
Missing await on postMessage call in error handler. This postMessage call should be awaited to ensure the error message is sent before resolving the deferred promise. Without await, the error message may not be posted if the function returns too quickly.
| })().catch((error: unknown) => { | |
| this.log.error('Failed to start hitandblow game', error); | |
| const errorText = | |
| error instanceof Error && error.stack !== undefined | |
| ? error.stack : String(error); | |
| this.postMessage({ | |
| })().catch(async (error: unknown) => { | |
| this.log.error('Failed to start hitandblow game', error); | |
| const errorText = | |
| error instanceof Error && error.stack !== undefined | |
| ? error.stack : String(error); | |
| await this.postMessage({ |
Refactor hitandblow bot to extend
ChannelLimitedBot, enabling execution in #tsgbot-games channel with automatic channel restriction and progress tracking.Changes
HitAndBlowBotclass extendingChannelLimitedBotCHANNEL_GAMESby default, with automatic redirect and message cleanup for disallowed channelsCHANNEL_SANDBOX, cleaned up on game completion (timeout or win)onWakeWord()with Deferred-based promise handlingthis.postMessage()for consistent username/iconFollows established pattern from anime (#1138), character-quiz (#1122), and achievement-quiz (#1124) migrations.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.