feat: add player 3 and 4 as coin input options#859
Conversation
📝 WalkthroughWalkthroughThis PR adds coin insertion support for players 3 and 4: new ZapScript handlers call ChangesPlayer 3 and 4 Coin Insertion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/zapscript/input.go`:
- Around line 266-276: Add unit tests that exercise the new command handlers
cmdCoinP3 and cmdCoinP4 to verify they map and execute correctly (not just
classification). Specifically, write tests that invoke the command
dispatch/mapping used in the package (the code path that resolves
"input.coinp3"/"input.coinp4" to handlers) and assert that cmdCoinP3 and
cmdCoinP4 call insertCoin with the expected player ID ("6") and propagate the
platforms.CmdResult/error behavior; reuse existing test helpers for creating a
mock platforms.Platform and platforms.CmdEnv and assert side effects/log output
as used by other handler tests in the package.
- Around line 267-276: cmdCoinP3 and cmdCoinP4 both call insertCoin with the
same key string "6", causing P3/P4 to behave like P2; update the calls in
functions cmdCoinP3 and cmdCoinP4 to pass distinct key identifiers for player 3
and player 4 (e.g., replace the "6" argument with the appropriate key codes such
as "7" for cmdCoinP3 and "8" for cmdCoinP4) so each handler forwards the correct
input to insertCoin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b875e71-7c2d-458a-a2e8-b1b2db952a5b
📒 Files selected for processing (3)
pkg/zapscript/commands_test.gopkg/zapscript/input.goscripts/taptui/taptui.sh
| //nolint:gocritic // single-use parameter in command handler | ||
| func cmdCoinP3(pl platforms.Platform, env platforms.CmdEnv) (platforms.CmdResult, error) { | ||
| log.Info().Msgf("inserting coin for player 3: %v", env.Cmd.Args) | ||
| return insertCoin(pl, env, "6") | ||
| } | ||
|
|
||
| //nolint:gocritic // single-use parameter in command handler | ||
| func cmdCoinP4(pl platforms.Platform, env platforms.CmdEnv) (platforms.CmdResult, error) { | ||
| log.Info().Msgf("inserting coin for player 4: %v", env.Cmd.Args) | ||
| return insertCoin(pl, env, "6") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add handler behavior tests for the new Go command paths.
The new handlers are added, but this PR only extends command classification tests; it does not validate handler execution/mapping behavior for input.coinp3/input.coinp4.
As per coding guidelines, **/*.go: "Write tests for all new code — see TESTING.md and pkg/testing/README.md".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/zapscript/input.go` around lines 266 - 276, Add unit tests that exercise
the new command handlers cmdCoinP3 and cmdCoinP4 to verify they map and execute
correctly (not just classification). Specifically, write tests that invoke the
command dispatch/mapping used in the package (the code path that resolves
"input.coinp3"/"input.coinp4" to handlers) and assert that cmdCoinP3 and
cmdCoinP4 call insertCoin with the expected player ID ("6") and propagate the
platforms.CmdResult/error behavior; reuse existing test helpers for creating a
mock platforms.Platform and platforms.CmdEnv and assert side effects/log output
as used by other handler tests in the package.
|
Actionable comments posted: 0 |
wizzomafizzo
changed the title
|
I'm happy to add this but it will need a little coordination with the go-zapscript repo which is where the command names are formally defined. |
2 participants
wanted to call from zapscript players 3 and 4 for insert coin
Summary by CodeRabbit
New Features
Tests