feat: improve mook takeout toast to show attacker and target names#365
Conversation
Changed "Took out 4 mooks" to "Brick Manly took out 4 Assassins" - attackHandlers.ts: Updated toast message to include attacker and mook names - combatHandlers.ts: Updated event description for consistency Fixes Fizzy #438
There was a problem hiding this comment.
Pull request overview
This PR improves the clarity of mook takedown messages by showing which attacker took out which mook type, replacing generic messages like "Took out 4 mooks" with more specific ones like "Brick Manly took out 4 Assassins".
Changes:
- Updated toast messages for mook takedowns to include attacker and mook character names
- Updated fight log descriptions for consistency with the new messaging format
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/encounters/attacks/combatHandlers.ts | Modified the event description for mook takedowns to include attacker name and mook character name instead of generic "mook"/"mooks" |
| src/components/encounters/attacks/attackHandlers.ts | Updated the toast message for mook takedowns to include attacker name and mook character name instead of generic "mook"/"mooks" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const description = | ||
| isMook && context.isMookTakedown | ||
| ? `${attacker.name} took out ${wounds} ${wounds === 1 ? "mook" : "mooks"}${defenseDesc}` | ||
| ? `${attacker.name} took out ${wounds} ${targetChar.name}${defenseDesc}` |
There was a problem hiding this comment.
The message may read awkwardly when the number of mooks doesn't match the grammatical number of the character name. For example, "Brick Manly took out 1 Zombies" is grammatically incorrect (should be "1 Zombie"). Mook character names are typically plural (e.g., "Zombies", "Assassins"), but the old code handled this by using "mook"/"mooks" with conditional pluralization based on the count. Consider adding logic to handle singular vs plural forms of the mook name, or use a more flexible phrasing that works regardless of count.
| ? `${attacker.name} took out ${wounds} ${targetChar.name}${defenseDesc}` | |
| ? `${attacker.name} took out ${wounds} of the ${targetChar.name}${defenseDesc}` |
| const isMook = CS.isMook(targetChar) | ||
| const toastMessage = isMook | ||
| ? `Took out ${effectiveWounds} ${effectiveWounds === 1 ? "mook" : "mooks"}` | ||
| ? `${attacker.name} took out ${effectiveWounds} ${targetChar.name}` |
There was a problem hiding this comment.
The toast message may read awkwardly when the number of mooks doesn't match the grammatical number of the character name. For example, "Brick Manly took out 1 Zombies" is grammatically incorrect (should be "1 Zombie"). Mook character names are typically plural (e.g., "Zombies", "Assassins"), but the old code handled this by using "mook"/"mooks" with conditional pluralization based on the count. Consider adding logic to handle singular vs plural forms of the mook name, or use a more flexible phrasing that works regardless of count.
| ? `${attacker.name} took out ${effectiveWounds} ${targetChar.name}` | |
| ? `${attacker.name} took out ${effectiveWounds} of the ${targetChar.name}` |
Summary
Changes
attackHandlers.ts: Updated toast message to include attacker name and mook character namecombatHandlers.ts: Updated event description for consistency in fight logsTest Plan
Fixes Fizzy #438