Make the history command more readable #448

2026-01-05T09:57:52Z

LunarTides commented

2026-01-05 09:57:52 +00:00

(Migrated from github.com)

Closes #445

Summary by CodeRabbit

New Features
- Added configurable readableHistory hooks for customizable, human-readable event history entries.
- Player entities now expose a readable representation for clearer display.
Refactor
- Unified and simplified history rendering, hide/reveal behavior, and history entry formatting for consistent output.
Bug Fixes
- History initialization no longer auto-inserts a default game-loop entry, preventing duplicate initial entries.
Chores
- Types expanded to support new history formatting hooks.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Closes #445  ## Summary by CodeRabbit * **New Features** * Added configurable readableHistory hooks for customizable, human-readable event history entries. * Player entities now expose a readable representation for clearer display. * **Refactor** * Unified and simplified history rendering, hide/reveal behavior, and history entry formatting for consistent output. * **Bug Fixes** * History initialization no longer auto-inserts a default game-loop entry, preventing duplicate initial entries. * **Chores** * Types expanded to support new history formatting hooks. ✏️ Tip: You can customize this high-level summary in your review settings.

coderabbitai[bot] commented

2026-01-05 09:57:57 +00:00

(Migrated from github.com)

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds a configurable per-event readableHistory mapping to produce human-readable history entries, removes Event.GameLoop (switching tick emission to Event.Dummy), updates history initialization and rendering logic, adjusts tests and interaction prompts, and adds Player.readable() plus a minor comment in Card.readable().

Changes

Cohort / File(s)	Summary
Tests `cards/Debug/inf-mana-47a7e94b.ts`, `cards/Examples/4/5-tick-hook.ts`	Replaced tick calls from `Event.GameLoop` with `Event.Dummy` and `undefined` payloads (tick trigger/source changed).
Config `config.ts`	Added `config.advanced.readableHistory` with per-event async handlers; expanded public imports to include `Alignment` and `TargetType`.
Types `src/types/event.ts`, `src/types/other.ts`	Removed `Event.GameLoop` enum member and its `EventValue` mapping; added `EventValue` import and typed `GameConfig.advanced.readableHistory`.
History & Commands `src/event.ts`, `src/commands.ts`	Initialize per-turn history as an empty array; introduced `handle(value, hide)` flow; integrated `readableHistory` callbacks when rendering history; adjusted hide/reveal checks and history formatting.
Runtime / Interaction `src/functions/interact.ts`	Switched tick emission from `Event.GameLoop` to `Event.Dummy`; unified input wrapper signature; changed prompt call/spacing in UI path.
Public API & Docs `src/player.ts`, `src/card.ts`	Added `Player.readable()` method; inserted a TODO comment in `Card.readable()`.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as Client
  participant Game as Game
  participant EventSystem as EventSystem
  participant Commands as Commands
  participant Config as Config

  Client->>Game: user action or tick trigger
  Game->>EventSystem: emit Event.Dummy (tick)
  EventSystem->>Game: update state
  Game->>EventSystem: addHistory(turn, eventKey, value)
  Client->>Commands: request history rendering
  Commands->>Config: lookup advanced.readableHistory[eventKey]?
  alt handler exists
    Commands->>Config: call handler(player, value, handle)
    Config-->>Commands: Promise<string> formattedEntry
  else no handler
    Commands->>Commands: call handle(value, hide) -> string
  end
  Commands->>Client: rendered history lines

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

LunarTides/Hearthstone.js#448 — adds/updates config.advanced.readableHistory handlers and history rendering changes; strong overlap with this PR's history formatting work.
LunarTides/Hearthstone.js#438 — modifies gameloop/tick emission and interaction flow similar to src/functions/interact.ts and test tick changes.

Suggested labels

breaking change

Poem

🐰
I hopped through history's new door,
Events now whisper words, not lore,
GameLoop hushed, Dummy gives a tap,
Players read lines on tidy map,
A carrot-cheer for clearer clap!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly aligns with the primary objective of making history command output more readable through human-readable event messages.
Linked Issues check	✅ Passed	The PR implements the core requirement of `#445`: adds readableHistory config mapping for human-readable event messages and integrates it into the history command.
Out of Scope Changes check	✅ Passed	Event system changes (removing GameLoop, adding Dummy event) and Player.readable() method are infrastructure changes supporting the readableHistory feature, which is within scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

> [!CAUTION] > ## Review failed > > The pull request is closed.   <details> <summary>📝 Walkthrough</summary> ## Walkthrough Adds a configurable per-event `readableHistory` mapping to produce human-readable history entries, removes `Event.GameLoop` (switching tick emission to `Event.Dummy`), updates history initialization and rendering logic, adjusts tests and interaction prompts, and adds `Player.readable()` plus a minor comment in `Card.readable()`. ## Changes | Cohort / File(s) | Summary | |---|---| | **Tests** `cards/Debug/inf-mana-47a7e94b.ts`, `cards/Examples/4/5-tick-hook.ts` | Replaced tick calls from `Event.GameLoop` with `Event.Dummy` and `undefined` payloads (tick trigger/source changed). | | **Config** `config.ts` | Added `config.advanced.readableHistory` with per-event async handlers; expanded public imports to include `Alignment` and `TargetType`. | | **Types** `src/types/event.ts`, `src/types/other.ts` | Removed `Event.GameLoop` enum member and its `EventValue` mapping; added `EventValue` import and typed `GameConfig.advanced.readableHistory`. | | **History & Commands** `src/event.ts`, `src/commands.ts` | Initialize per-turn history as an empty array; introduced `handle(value, hide)` flow; integrated `readableHistory` callbacks when rendering history; adjusted hide/reveal checks and history formatting. | | **Runtime / Interaction** `src/functions/interact.ts` | Switched tick emission from `Event.GameLoop` to `Event.Dummy`; unified input wrapper signature; changed prompt call/spacing in UI path. | | **Public API & Docs** `src/player.ts`, `src/card.ts` | Added `Player.readable()` method; inserted a TODO comment in `Card.readable()`. | ## Sequence Diagram(s) ```mermaid sequenceDiagram autonumber participant Client as Client participant Game as Game participant EventSystem as EventSystem participant Commands as Commands participant Config as Config Client->>Game: user action or tick trigger Game->>EventSystem: emit Event.Dummy (tick) EventSystem->>Game: update state Game->>EventSystem: addHistory(turn, eventKey, value) Client->>Commands: request history rendering Commands->>Config: lookup advanced.readableHistory[eventKey]? alt handler exists Commands->>Config: call handler(player, value, handle) Config-->>Commands: Promise<string> formattedEntry else no handler Commands->>Commands: call handle(value, hide) -> string end Commands->>Client: rendered history lines ``` ## Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes ## Possibly related PRs - LunarTides/Hearthstone.js#448 — adds/updates `config.advanced.readableHistory` handlers and history rendering changes; strong overlap with this PR's history formatting work. - LunarTides/Hearthstone.js#438 — modifies gameloop/tick emission and interaction flow similar to `src/functions/interact.ts` and test tick changes. ## Suggested labels `breaking change` ## Poem > 🐰 > I hopped through history's new door, > Events now whisper words, not lore, > GameLoop hushed, Dummy gives a tap, > Players read lines on tidy map, > A carrot-cheer for clearer clap! </details>   <details> <summary>🚥 Pre-merge checks | ✅ 5</summary> <details> <summary>✅ Passed checks (5 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title check | ✅ Passed | The PR title directly aligns with the primary objective of making history command output more readable through human-readable event messages. | | Linked Issues check | ✅ Passed | The PR implements the core requirement of `#445`: adds readableHistory config mapping for human-readable event messages and integrates it into the history command. | | Out of Scope Changes check | ✅ Passed | Event system changes (removing GameLoop, adding Dummy event) and Player.readable() method are infrastructure changes supporting the readableHistory feature, which is within scope. | | Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. | </details> ✏️ Tip: You can configure your own custom pre-merge checks in the settings. </details>   <details> <summary>✨ Finishing touches</summary> - [ ]  📝 Generate docstrings </details>   --- Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=LunarTides/Hearthstone.js&utm_content=448)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> Comment `@coderabbitai help` to get the list of available commands and usage tips.

coderabbitai[bot] (Migrated from github.com) reviewed

2026-01-20 13:50:40 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@config.ts`:
- Around line 465-466: In the Event.SpellDealsDamage template string the closing
bold tag is malformed; update the arrow function handler for
Event.SpellDealsDamage (parameters plr, [target, amount], handle) so the
returned string uses a proper closing tag — change `<b>${amount}<b>` to
`<b>${amount}</b>` in the template returned by that function to correctly close
the HTML bold tag.
- Around line 390-391: The HealthRestored message is misleading because
Event.HealthRestored handlers receive the new total health, not the delta;
update the template string in the Event.HealthRestored handler (the arrow for
[Event.HealthRestored]) to say "restored to <b>${await handle(value)}</b>
health" (or otherwise compute delta if you prefer) so the text matches the value
passed by the event.

🧹 Nitpick comments (2)

src/card.ts (1)

2074-2074: Noted: TODO for debug info placement.

This TODO comment is clear. Would you like me to open an issue to track this task, or would you prefer to address it in a follow-up PR?
src/commands.ts (1)
522-530: Type assertion workaround is acceptable but consider documenting the reason.

The as never cast on line 525 is a common workaround for TypeScript's limitation when the event key type cannot be narrowed within a loop. The TODO comment acknowledges this, but consider expanding it to explain why it's needed (TypeScript can't correlate the key variable with the generic E in readableHistory[key]).

The fallback logic correctly uses the processed newValue array when no custom handler exists.
📝 Suggested documentation improvement
 let entry = await game.config.advanced.readableHistory[key]?.(
 player,
- // TODO: Use of never.
+ // TypeScript cannot narrow `key` to correlate with EventValue<E> here,
+ // so we cast to `never` to satisfy the generic constraint.
 value as never,
 (value: unknown, hide?: boolean) => handle(value, hide ?? shouldHide),
 );

**Actionable comments posted: 2** <details> <summary>🤖 Fix all issues with AI agents</summary> ``` In `@config.ts`: - Around line 465-466: In the Event.SpellDealsDamage template string the closing bold tag is malformed; update the arrow function handler for Event.SpellDealsDamage (parameters plr, [target, amount], handle) so the returned string uses a proper closing tag — change `${amount}` to `${amount}` in the template returned by that function to correctly close the HTML bold tag. - Around line 390-391: The HealthRestored message is misleading because Event.HealthRestored handlers receive the new total health, not the delta; update the template string in the Event.HealthRestored handler (the arrow for [Event.HealthRestored]) to say "restored to ${await handle(value)} health" (or otherwise compute delta if you prefer) so the text matches the value passed by the event. ``` </details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/card.ts (1)</summary><blockquote> `2074-2074`: **Noted: TODO for debug info placement.** This TODO comment is clear. Would you like me to open an issue to track this task, or would you prefer to address it in a follow-up PR? </blockquote></details> <details> <summary>src/commands.ts (1)</summary><blockquote> `522-530`: **Type assertion workaround is acceptable but consider documenting the reason.** The `as never` cast on line 525 is a common workaround for TypeScript's limitation when the event key type cannot be narrowed within a loop. The TODO comment acknowledges this, but consider expanding it to explain *why* it's needed (TypeScript can't correlate the `key` variable with the generic `E` in `readableHistory[key]`). The fallback logic correctly uses the processed `newValue` array when no custom handler exists. <details> <summary>📝 Suggested documentation improvement</summary> ```diff let entry = await game.config.advanced.readableHistory[key]?.( player, - // TODO: Use of never. + // TypeScript cannot narrow `key` to correlate with EventValue<E> here, + // so we cast to `never` to satisfy the generic constraint. value as never, (value: unknown, hide?: boolean) => handle(value, hide ?? shouldHide), ); ``` </details> </blockquote></details> </blockquote></details>

config.ts

					
				@ -365,6 +371,127 @@ export const config: GameConfig = {

							Event.AddCardToHand,

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all Event.HealthRestored usages
rg -n 'Event\.HealthRestored' -A 2 -B 2 --type=ts

Repository: LunarTides/Hearthstone.js

Length of output: 1147

🏁 Script executed:

#!/bin/bash
# Also check the event type definition
rg -n 'HealthRestored' src/types/event.ts -A 3 -B 3

Repository: LunarTides/Hearthstone.js

Length of output: 505

🏁 Script executed:

#!/bin/bash
# Search for where health restore events are broadcast to see what value is passed
rg -n 'broadcast.*[Hh]ealth' --type=ts -A 1 -B 1 | head -100

Repository: LunarTides/Hearthstone.js

Length of output: 235

Fix misleading health restoration message text.

The message says "restored ${value} health", which implies value is the amount restored (delta). However, examining the broadcasts in src/card.ts (lines 914, 919), the HealthRestored event passes the resulting health total (this.maxHealth or this.health), not the delta. The type comment in src/types/event.ts (lines 143-146) correctly reflects this as "The amount of health after restore."

Change the message to "restored to ${value} health" or calculate and pass the delta as the event value instead.

🤖 Prompt for AI Agents

In `@config.ts` around lines 390 - 391, The HealthRestored message is misleading
because Event.HealthRestored handlers receive the new total health, not the
delta; update the template string in the Event.HealthRestored handler (the arrow
for [Event.HealthRestored]) to say "restored to <b>${await handle(value)}</b>
health" (or otherwise compute delta if you prefer) so the text matches the value
passed by the event.

✅ Addressed in commit f9712c9

_⚠️ Potential issue_ | _🟡 Minor_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash # Find all Event.HealthRestored usages rg -n 'Event\.HealthRestored' -A 2 -B 2 --type=ts ``` Repository: LunarTides/Hearthstone.js Length of output: 1147 --- 🏁 Script executed: ```shell #!/bin/bash # Also check the event type definition rg -n 'HealthRestored' src/types/event.ts -A 3 -B 3 ``` Repository: LunarTides/Hearthstone.js Length of output: 505 --- 🏁 Script executed: ```shell #!/bin/bash # Search for where health restore events are broadcast to see what value is passed rg -n 'broadcast.*[Hh]ealth' --type=ts -A 1 -B 1 | head -100 ``` Repository: LunarTides/Hearthstone.js Length of output: 235 --- </details> **Fix misleading health restoration message text.** The message says `"restored ${value} health"`, which implies `value` is the amount restored (delta). However, examining the broadcasts in `src/card.ts` (lines 914, 919), the `HealthRestored` event passes the resulting health total (`this.maxHealth` or `this.health`), not the delta. The type comment in `src/types/event.ts` (lines 143-146) correctly reflects this as "The amount of health after restore." Change the message to `"restored to ${value} health"` or calculate and pass the delta as the event value instead. <details> <summary>🤖 Prompt for AI Agents</summary> ``` In `@config.ts` around lines 390 - 391, The HealthRestored message is misleading because Event.HealthRestored handlers receive the new total health, not the delta; update the template string in the Event.HealthRestored handler (the arrow for [Event.HealthRestored]) to say "restored to ${await handle(value)} health" (or otherwise compute delta if you prefer) so the text matches the value passed by the event. ``` </details>   ✅ Addressed in commit f9712c9

⚠️ Potential issue | 🟡 Minor

Unclosed bold tag.

The closing tag is missing a forward slash: ${amount} should be ${amount}.

🔧 Proposed fix

 [Event.SpellDealsDamage]: async (plr, [target, amount], handle) =>
-  `${await handle(target)} dealt <b>${amount}<b> damage`,
+  `${await handle(target)} dealt <b>${amount}</b> damage`,

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

			[Event.SpellDealsDamage]: async (plr, [target, amount], handle) =>
				`${await handle(target)} dealt <b>${amount}</b> damage`,

🤖 Prompt for AI Agents

In `@config.ts` around lines 465 - 466, In the Event.SpellDealsDamage template
string the closing bold tag is malformed; update the arrow function handler for
Event.SpellDealsDamage (parameters plr, [target, amount], handle) so the
returned string uses a proper closing tag — change `<b>${amount}<b>` to
`<b>${amount}</b>` in the template returned by that function to correctly close
the HTML bold tag.

✅ Addressed in commit f9712c9

_⚠️ Potential issue_ | _🟡 Minor_ **Unclosed bold tag.** The closing tag is missing a forward slash: `${amount}` should be `${amount}`. <details> <summary>🔧 Proposed fix</summary> ```diff [Event.SpellDealsDamage]: async (plr, [target, amount], handle) => - `${await handle(target)} dealt ${amount} damage`, + `${await handle(target)} dealt ${amount} damage`, ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion [Event.SpellDealsDamage]: async (plr, [target, amount], handle) => `${await handle(target)} dealt ${amount} damage`, ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` In `@config.ts` around lines 465 - 466, In the Event.SpellDealsDamage template string the closing bold tag is malformed; update the arrow function handler for Event.SpellDealsDamage (parameters plr, [target, amount], handle) so the returned string uses a proper closing tag — change `${amount}` to `${amount}` in the template returned by that function to correctly close the HTML bold tag. ``` </details>   ✅ Addressed in commit f9712c9

coderabbitai[bot] (Migrated from github.com) reviewed

2026-01-20 14:05:12 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands.ts (1)
399-436: Reveal detection misses cards embedded in tuple event values.

Events like RevealCard, DamageCard, etc. store cards inside arrays, but the reveal scan only checks direct Card values. That can leave previously revealed cards shown as Hidden.
🧩 Fix by scanning arrays for matching cards
-// If it is not a card
-if (!(newValue instanceof Card)) {
-	continue;
-}
-
-if (value.uuid !== newValue.uuid) {
-	continue;
-}
-
-// The card has been revealed.
-revealed = true;
+const matches =
+	newValue instanceof Card
+		? newValue.uuid === value.uuid
+		: Array.isArray(newValue)
+			? newValue.some(
+					(item) => item instanceof Card && item.uuid === value.uuid,
+				)
+			: false;
+
+if (!matches) {
+	continue;
+}
+
+// The card has been revealed.
+revealed = true;

🤖 Fix all issues with AI agents

In `@config.ts`:
- Around line 375-494: The TargetSelectionStarts readableHistory entry uses
truthy checks for flags.targetType and flags.alignment which will treat valid
enum values 0 as missing; update the conditional expressions in the
TargetSelectionStarts handler to use explicit !== undefined checks (e.g.,
flags.targetType !== undefined and flags.alignment !== undefined) when deciding
whether to render TargetType[flags.targetType] or Alignment[flags.alignment] so
the fallback "All" is only used for undefined values; keep the rest of the
handler (plr, prompt, host, handle usage) unchanged.

🧹 Nitpick comments (1)

src/commands.ts (1)
377-384: Prefer Player.readable() for consistency.

This aligns history formatting with Card.readable() and future-proofs player formatting.
♻️ Suggested tweak
-if (value instanceof Player) {
-	return value.getName();
-}
+if (value instanceof Player) {
+	return await value.readable();
+}

**Actionable comments posted: 1** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > > <details> > <summary>⚠️ Outside diff range comments (1)</summary><blockquote> > > <details> > <summary>src/commands.ts (1)</summary><blockquote> > > `399-436`: **Reveal detection misses cards embedded in tuple event values.** > > Events like `RevealCard`, `DamageCard`, etc. store cards inside arrays, but the reveal scan only checks direct `Card` values. That can leave previously revealed cards shown as `Hidden`. > > > <details> > <summary>🧩 Fix by scanning arrays for matching cards</summary> > > ```diff > -// If it is not a card > -if (!(newValue instanceof Card)) { > - continue; > -} > - > -if (value.uuid !== newValue.uuid) { > - continue; > -} > - > -// The card has been revealed. > -revealed = true; > +const matches = > + newValue instanceof Card > + ? newValue.uuid === value.uuid > + : Array.isArray(newValue) > + ? newValue.some( > + (item) => item instanceof Card && item.uuid === value.uuid, > + ) > + : false; > + > +if (!matches) { > + continue; > +} > + > +// The card has been revealed. > +revealed = true; > ``` > </details> > > </blockquote></details> > > </blockquote></details> <details> <summary>🤖 Fix all issues with AI agents</summary> ``` In `@config.ts`: - Around line 375-494: The TargetSelectionStarts readableHistory entry uses truthy checks for flags.targetType and flags.alignment which will treat valid enum values 0 as missing; update the conditional expressions in the TargetSelectionStarts handler to use explicit !== undefined checks (e.g., flags.targetType !== undefined and flags.alignment !== undefined) when deciding whether to render TargetType[flags.targetType] or Alignment[flags.alignment] so the fallback "All" is only used for undefined values; keep the rest of the handler (plr, prompt, host, handle usage) unchanged. ``` </details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/commands.ts (1)</summary><blockquote> `377-384`: **Prefer `Player.readable()` for consistency.** This aligns history formatting with `Card.readable()` and future-proofs player formatting. <details> <summary>♻️ Suggested tweak</summary> ```diff -if (value instanceof Player) { - return value.getName(); -} +if (value instanceof Player) { + return await value.readable(); +} ``` </details> </blockquote></details> </blockquote></details>

config.ts

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n "enum Alignment|enum TargetType" --type=ts

Repository: LunarTides/Hearthstone.js

Length of output: 161

🏁 Script executed:

sed -n '75,95p' src/types/other.ts

Repository: LunarTides/Hearthstone.js

Length of output: 371

🏁 Script executed:

sed -n '437,447p' config.ts

Repository: LunarTides/Hearthstone.js

Length of output: 467

🏁 Script executed:

rg -n "Event.TargetSelectionStarts" config.ts -A 5

Repository: LunarTides/Hearthstone.js

Length of output: 562

Replace enum lookups with explicit !== undefined checks in TargetSelectionStarts.

The current truthiness checks will fail when flags.targetType or flags.alignment is 0, which are valid enum values (TargetType.Player and Alignment.Friendly respectively). This causes the fallback "All" to display incorrectly.

Fix: Use explicit undefined checks

[Event.TargetSelectionStarts]: async (
	plr,
	[prompt, host, flags],
	handle,
) =>
	`${await handle(plr)} started selecting a target. [Prompt: "${prompt}", Host: ${host ? await handle(host) : "Game"}, Type: ${flags.targetType !== undefined ? TargetType[flags.targetType] : "All"}, Alignment: ${flags.alignment !== undefined ? Alignment[flags.alignment] : "All"}]`,

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

		/**
		 * For each event here, it will show the return value of the function in the history command.
		 *
		 * The `handle` function should be called when dealing with the value. Don't change the `hide` parameter unless you have a good reason to.
		 */
		readableHistory: {
			[Event.FatalDamage]: async (plr, value, handle) =>
				`${await handle(plr)} was dealt fatal damage`,

			[Event.EndTurn]: async (plr, value, handle) =>
				`${await handle(plr)} ended their turn`,

			[Event.StartTurn]: async (plr, value, handle) =>
				`${await handle(plr)} started their turn`,

			[Event.HealthRestored]: async (plr, value, handle) =>
				`${await handle(plr)} restored to <b>${await handle(value)}</b> health`,

			[Event.UnspentMana]: async (plr, value, handle) =>
				`${await handle(plr)} ended their turn with <b>${await handle(value)}</b> unspent mana`,

			[Event.GainOverload]: async (plr, value, handle) =>
				`${await handle(plr)} gained <b>${await handle(value)}</b> overload`,

			[Event.GainHeroAttack]: async (plr, value, handle) =>
				`${await handle(plr)} gained <b>${await handle(value)}</b> attack`,

			[Event.TakeDamage]: async (plr, value, handle) =>
				`${await handle(plr)} took <b>${await handle(value)}</b> damage`,

			[Event.PlayCard]: async (plr, value, handle) =>
				`${await handle(plr)} played ${await handle(value)}`,

			[Event.PlayCardUnsafe]: async (plr, value, handle) =>
				`${await handle(plr)} is trying to play a ${await handle(value)}`,

			[Event.SummonCard]: async (plr, value, handle) =>
				`${await handle(plr)} summoned a ${await handle(value)}`,

			[Event.DestroyCard]: async (plr, value, handle) =>
				`${await handle(value)} was destroyed`,

			[Event.DamageCard]: async (plr, [card, amount], handle) =>
				`${await handle(card)} was dealt <b>${await handle(amount)}</b> damage`,

			[Event.SilenceCard]: async (plr, value, handle) =>
				`${await handle(value)} was silenced`,

			[Event.DiscardCard]: async (plr, value, handle) =>
				`${await handle(value)} was discarded`,

			[Event.CancelCard]: async (plr, [card, ability], handle) =>
				`${await handle(card)}'s <b>${await handle(ability)}</b> was cancelled`,

			[Event.TradeCard]: async (plr, value, handle) =>
				`${await handle(value)} was traded`,

			[Event.ForgeCard]: async (plr, value, handle) =>
				`${await handle(value)} was forged`,

			[Event.FreezeCard]: async (plr, value, handle) =>
				`${await handle(value)} was frozen`,

			[Event.CreateCard]: async (plr, value, handle) =>
				`${await handle(value)} was created`,

			[Event.RevealCard]: async (plr, [card, reason], handle) =>
				`${await handle(card)} was revealed due to ${reason}`,

			[Event.BurnCard]: async (plr, value, handle) =>
				`${await handle(value)} was burned due to a lack of hand space`,

			[Event.Titan]: async (plr, [titan, ability], handle) =>
				`${await handle(titan)}'s titan ability (${await handle(ability)}) was triggered`,

			[Event.AddCardToDeck]: async (plr, value, handle) =>
				`${await handle(value)} was added to ${await handle(plr)}'s deck`,

			[Event.AddCardToHand]: async (plr, value, handle) =>
				`${await handle(value)} was added to ${await handle(plr)}'s hand`,

			[Event.DrawCard]: async (plr, value, handle) =>
				`${await handle(plr)} drew ${await handle(value)}`,

			[Event.ChangeLocation]: async (plr, [card, location], handle) =>
				`${await handle(card)}'s location was changed to <b>${await handle(location)}</b>`,

			[Event.ChangeHero]: async (plr, [oldHero, newHero], handle) =>
				`${await handle(plr)}'s hero has become ${await handle(newHero)}`,

			[Event.SpellDealsDamage]: async (plr, [target, amount], handle) =>
				`${await handle(target)} dealt <b>${amount}</b> damage`,

			[Event.Attack]: async (plr, [attacker, target, flags], handle) =>
				`${await handle(attacker)} attacked ${await handle(target)}`,

			[Event.HeroPower]: async (plr, value, handle) =>
				`${await handle(plr)} used their hero power`,

			[Event.TargetSelectionStarts]: async (
				plr,
				[prompt, host, flags],
				handle,
			) =>
				`${await handle(plr)} started selecting a target. [Prompt: "${prompt}", Host: ${host ? await handle(host) : "Game"}, Type: ${flags.targetType !== undefined ? TargetType[flags.targetType] : "All"}, Alignment: ${flags.alignment !== undefined ? Alignment[flags.alignment] : "All"}]`,

			[Event.TargetSelected]: async (plr, [host, target], handle) =>
				`${await handle(plr)} selected ${await handle(target)} [Host: ${host ? await handle(host) : "Game"}]`,

			[Event.CardEvent]: async (plr, [card, event], handle) =>
				`${await handle(card)} said: ${event}`,

			[Event.Dummy]: async (plr, value, handle) => `Dummy Event (Test)`,

			[Event.Eval]: async (plr, value, handle) =>
				`${await handle(plr)} eval'd: ${await handle(value)}`,

			[Event.Input]: async (plr, value, handle) =>
				`${await handle(plr)} typed: ${await handle(value)}`,
		},

🤖 Prompt for AI Agents

In `@config.ts` around lines 375 - 494, The TargetSelectionStarts readableHistory
entry uses truthy checks for flags.targetType and flags.alignment which will
treat valid enum values 0 as missing; update the conditional expressions in the
TargetSelectionStarts handler to use explicit !== undefined checks (e.g.,
flags.targetType !== undefined and flags.alignment !== undefined) when deciding
whether to render TargetType[flags.targetType] or Alignment[flags.alignment] so
the fallback "All" is only used for undefined values; keep the rest of the
handler (plr, prompt, host, handle usage) unchanged.

✅ Addressed in commit 2fca7ec

_⚠️ Potential issue_ | _🔴 Critical_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell rg -n "enum Alignment|enum TargetType" --type=ts ``` Repository: LunarTides/Hearthstone.js Length of output: 161 --- 🏁 Script executed: ```shell sed -n '75,95p' src/types/other.ts ``` Repository: LunarTides/Hearthstone.js Length of output: 371 --- 🏁 Script executed: ```shell sed -n '437,447p' config.ts ``` Repository: LunarTides/Hearthstone.js Length of output: 467 --- 🏁 Script executed: ```shell rg -n "Event.TargetSelectionStarts" config.ts -A 5 ``` Repository: LunarTides/Hearthstone.js Length of output: 562 --- </details> **Replace enum lookups with explicit `!== undefined` checks in `TargetSelectionStarts`.** The current truthiness checks will fail when `flags.targetType` or `flags.alignment` is `0`, which are valid enum values (`TargetType.Player` and `Alignment.Friendly` respectively). This causes the fallback `"All"` to display incorrectly. <details> <summary>Fix: Use explicit undefined checks</summary> ```diff [Event.TargetSelectionStarts]: async ( plr, [prompt, host, flags], handle, ) => `${await handle(plr)} started selecting a target. [Prompt: "${prompt}", Host: ${host ? await handle(host) : "Game"}, Type: ${flags.targetType !== undefined ? TargetType[flags.targetType] : "All"}, Alignment: ${flags.alignment !== undefined ? Alignment[flags.alignment] : "All"}]`, ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion /** * For each event here, it will show the return value of the function in the history command. * * The `handle` function should be called when dealing with the value. Don't change the `hide` parameter unless you have a good reason to. */ readableHistory: { [Event.FatalDamage]: async (plr, value, handle) => `${await handle(plr)} was dealt fatal damage`, [Event.EndTurn]: async (plr, value, handle) => `${await handle(plr)} ended their turn`, [Event.StartTurn]: async (plr, value, handle) => `${await handle(plr)} started their turn`, [Event.HealthRestored]: async (plr, value, handle) => `${await handle(plr)} restored to ${await handle(value)} health`, [Event.UnspentMana]: async (plr, value, handle) => `${await handle(plr)} ended their turn with ${await handle(value)} unspent mana`, [Event.GainOverload]: async (plr, value, handle) => `${await handle(plr)} gained ${await handle(value)} overload`, [Event.GainHeroAttack]: async (plr, value, handle) => `${await handle(plr)} gained ${await handle(value)} attack`, [Event.TakeDamage]: async (plr, value, handle) => `${await handle(plr)} took ${await handle(value)} damage`, [Event.PlayCard]: async (plr, value, handle) => `${await handle(plr)} played ${await handle(value)}`, [Event.PlayCardUnsafe]: async (plr, value, handle) => `${await handle(plr)} is trying to play a ${await handle(value)}`, [Event.SummonCard]: async (plr, value, handle) => `${await handle(plr)} summoned a ${await handle(value)}`, [Event.DestroyCard]: async (plr, value, handle) => `${await handle(value)} was destroyed`, [Event.DamageCard]: async (plr, [card, amount], handle) => `${await handle(card)} was dealt ${await handle(amount)} damage`, [Event.SilenceCard]: async (plr, value, handle) => `${await handle(value)} was silenced`, [Event.DiscardCard]: async (plr, value, handle) => `${await handle(value)} was discarded`, [Event.CancelCard]: async (plr, [card, ability], handle) => `${await handle(card)}'s ${await handle(ability)} was cancelled`, [Event.TradeCard]: async (plr, value, handle) => `${await handle(value)} was traded`, [Event.ForgeCard]: async (plr, value, handle) => `${await handle(value)} was forged`, [Event.FreezeCard]: async (plr, value, handle) => `${await handle(value)} was frozen`, [Event.CreateCard]: async (plr, value, handle) => `${await handle(value)} was created`, [Event.RevealCard]: async (plr, [card, reason], handle) => `${await handle(card)} was revealed due to ${reason}`, [Event.BurnCard]: async (plr, value, handle) => `${await handle(value)} was burned due to a lack of hand space`, [Event.Titan]: async (plr, [titan, ability], handle) => `${await handle(titan)}'s titan ability (${await handle(ability)}) was triggered`, [Event.AddCardToDeck]: async (plr, value, handle) => `${await handle(value)} was added to ${await handle(plr)}'s deck`, [Event.AddCardToHand]: async (plr, value, handle) => `${await handle(value)} was added to ${await handle(plr)}'s hand`, [Event.DrawCard]: async (plr, value, handle) => `${await handle(plr)} drew ${await handle(value)}`, [Event.ChangeLocation]: async (plr, [card, location], handle) => `${await handle(card)}'s location was changed to ${await handle(location)}`, [Event.ChangeHero]: async (plr, [oldHero, newHero], handle) => `${await handle(plr)}'s hero has become ${await handle(newHero)}`, [Event.SpellDealsDamage]: async (plr, [target, amount], handle) => `${await handle(target)} dealt ${amount} damage`, [Event.Attack]: async (plr, [attacker, target, flags], handle) => `${await handle(attacker)} attacked ${await handle(target)}`, [Event.HeroPower]: async (plr, value, handle) => `${await handle(plr)} used their hero power`, [Event.TargetSelectionStarts]: async ( plr, [prompt, host, flags], handle, ) => `${await handle(plr)} started selecting a target. [Prompt: "${prompt}", Host: ${host ? await handle(host) : "Game"}, Type: ${flags.targetType !== undefined ? TargetType[flags.targetType] : "All"}, Alignment: ${flags.alignment !== undefined ? Alignment[flags.alignment] : "All"}]`, [Event.TargetSelected]: async (plr, [host, target], handle) => `${await handle(plr)} selected ${await handle(target)} [Host: ${host ? await handle(host) : "Game"}]`, [Event.CardEvent]: async (plr, [card, event], handle) => `${await handle(card)} said: ${event}`, [Event.Dummy]: async (plr, value, handle) => `Dummy Event (Test)`, [Event.Eval]: async (plr, value, handle) => `${await handle(plr)} eval'd: ${await handle(value)}`, [Event.Input]: async (plr, value, handle) => `${await handle(plr)} typed: ${await handle(value)}`, }, ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` In `@config.ts` around lines 375 - 494, The TargetSelectionStarts readableHistory entry uses truthy checks for flags.targetType and flags.alignment which will treat valid enum values 0 as missing; update the conditional expressions in the TargetSelectionStarts handler to use explicit !== undefined checks (e.g., flags.targetType !== undefined and flags.alignment !== undefined) when deciding whether to render TargetType[flags.targetType] or Alignment[flags.alignment] so the fallback "All" is only used for undefined values; keep the rest of the handler (plr, prompt, host, handle usage) unchanged. ``` </details>   ✅ Addressed in commit 2fca7ec

coderabbitai[bot] (Migrated from github.com) reviewed

2026-01-20 14:18:05 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/commands.ts`:
- Around line 377-384: In the handle function, the branch handling Player
instances calls the async method Player.readable() but doesn't await it, causing
a Promise to be returned instead of the resolved string; update the Player
branch in handle (the async function handle(value: unknown, hide: boolean):
Promise<string>) to await value.readable() (i.e., return await value.readable())
so the function returns the resolved string as declared.

🧹 Nitpick comments (1)

src/commands.ts (1)
510-520: Consider typing newValue as string[] for clarity.

The newValue array is populated exclusively with strings returned from handle(), but it's typed as unknown[]. A more precise type would improve readability and type safety.
♻️ Suggested improvement
-				const newValue: unknown[] = [];
+				const newValue: string[] = [];

**Actionable comments posted: 1** <details> <summary>🤖 Fix all issues with AI agents</summary> ``` In `@src/commands.ts`: - Around line 377-384: In the handle function, the branch handling Player instances calls the async method Player.readable() but doesn't await it, causing a Promise to be returned instead of the resolved string; update the Player branch in handle (the async function handle(value: unknown, hide: boolean): Promise<string>) to await value.readable() (i.e., return await value.readable()) so the function returns the resolved string as declared. ``` </details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/commands.ts (1)</summary><blockquote> `510-520`: **Consider typing `newValue` as `string[]` for clarity.** The `newValue` array is populated exclusively with strings returned from `handle()`, but it's typed as `unknown[]`. A more precise type would improve readability and type safety. <details> <summary>♻️ Suggested improvement</summary> ```diff - const newValue: unknown[] = []; + const newValue: string[] = []; ``` </details> </blockquote></details> </blockquote></details>

src/commands.ts

					
				@ -395,2 +392,2 @@

							if (!hide || value.owner === player) {

								return await showCard(value);

							if (!hide || value.owner === game.player) {

								return await value.readable();

⚠️ Potential issue | 🔴 Critical

Missing await for Player.readable() call.

Player.readable() is an async method (returns Promise<string> per the relevant snippet), but the call on line 379 is not awaited. This will return a Promise object instead of the resolved string.

🐛 Proposed fix

 		const handle = async (value: unknown, hide: boolean): Promise<string> => {
 			if (value instanceof Player) {
-				return value.readable();
+				return await value.readable();
 			}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

		const handle = async (value: unknown, hide: boolean): Promise<string> => {
			if (value instanceof Player) {
				return await value.readable();
			}

			if (typeof value === "string") {
				return value;
			}

🤖 Prompt for AI Agents

In `@src/commands.ts` around lines 377 - 384, In the handle function, the branch
handling Player instances calls the async method Player.readable() but doesn't
await it, causing a Promise to be returned instead of the resolved string;
update the Player branch in handle (the async function handle(value: unknown,
hide: boolean): Promise<string>) to await value.readable() (i.e., return await
value.readable()) so the function returns the resolved string as declared.

✅ Addressed in commit bcdcabf

_⚠️ Potential issue_ | _🔴 Critical_ **Missing `await` for `Player.readable()` call.** `Player.readable()` is an async method (returns `Promise<string>` per the relevant snippet), but the call on line 379 is not awaited. This will return a Promise object instead of the resolved string. <details> <summary>🐛 Proposed fix</summary> ```diff const handle = async (value: unknown, hide: boolean): Promise<string> => { if (value instanceof Player) { - return value.readable(); + return await value.readable(); } ``` </details>  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion const handle = async (value: unknown, hide: boolean): Promise<string> => { if (value instanceof Player) { return await value.readable(); } if (typeof value === "string") { return value; } ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` In `@src/commands.ts` around lines 377 - 384, In the handle function, the branch handling Player instances calls the async method Player.readable() but doesn't await it, causing a Promise to be returned instead of the resolved string; update the Player branch in handle (the async function handle(value: unknown, hide: boolean): Promise<string>) to await value.readable() (i.e., return await value.readable()) so the function returns the resolved string as declared. ``` </details>   ✅ Addressed in commit bcdcabf

Sign in to join this conversation.

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

LunarTides/Hearthstone.js!448

				`@ -365,6 +371,127 @@ export const config: GameConfig = {`
				`Event.AddCardToHand,`

Rows
Columns