-
Notifications
You must be signed in to change notification settings - Fork 0
Code rabbit test #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
JeremyDuncan
wants to merge
5
commits into
main
Choose a base branch
from
code-rabbit-test
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
77c109a
codex: modulized js code and updated styling
JeremyDuncan 61b02f1
codex: updated js logic and styling
JeremyDuncan 45db77d
codex: breaking change!
JeremyDuncan a9e2b18
codex: breaking change!
JeremyDuncan 5c2d5d6
code-rabbit: added comment
JeremyDuncan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Repository Guidelines | ||
|
|
||
| ## Project Structure & Module Organization | ||
| `index.html` is the single-page shell; it links the modern styling in `battleship.css` and boots `battleship.js` as an ES module. Gameplay logic lives under `src/`: `game.js` orchestrates turns and win states, `board.js` tracks ship placement and attacks, `ship.js` models individual vessels, `ai-controller.js` handles smart CPU targeting, and `ui-controller.js` updates the DOM. Shared constants (ship specs, orientations, board size) are defined in `src/constants.js` so both players stay in sync. `battleship.js` wires the controllers together, exposes the global bridge required by the existing `onclick` handlers, and inserts the current year into the footer. Static assets (splash/explosion SVGs, icons, screenshots) live in `images/`; prefer `.webp` for screenshots and keep SVGs optimized (`svgo` or similar) when adding new art. | ||
|
|
||
| ## Build, Test, and Development Commands | ||
| - `npx http-server . --port 3000` serves the project with correct relative paths and caching headers; use it during active development. | ||
| - `python3 -m http.server 3000` provides a zero-dependency fallback if Node.js is unavailable. | ||
| - `open index.html` (or your OS equivalent) is fine for quick visual checks, but an HTTP server avoids asset/CORS issues and mirrors production loading better. | ||
|
|
||
| ## Coding Style & Naming Conventions | ||
| Follow the ES module pattern already in place: default to `const`/`let`, keep classes in their own files, and export the minimal surface each module needs. JavaScript uses two-space indentation and descriptive method names (`placePlayerShip`, `handleAttackResult`). CSS also uses two spaces and kebab-case selectors; leverage the variables at the top of `battleship.css`—especially `--cell-size` for board square sizing—rather than hard-coding pixels. DOM IDs remain camelCase (`remove-on-start` is the lone kebab-case ID for legacy reasons); keep new classes prefixed with the component they style (`.player-board`, `.ship-select`) to make overrides predictable. | ||
|
|
||
| ## Testing Guidelines | ||
| There is no automated suite yet, so rely on manual QA. Serve the site, then walk through ship placement, game start, repeated attack loops, and both win and loss flows. Confirm the hover preview outlines the full ship footprint (blue when valid, red when out of bounds or overlapping) before you click, the CPU AI switches from random attacks to focused hunts after a hit, and the retry button reloads cleanly. Validate responsive breakpoints at 1280px, 1024px, 768px, and 375px widths—the UI should keep both boards visible side by side on desktop and wrap sensibly on smaller screens. Capture GIFs or annotated screenshots for regressions and attach them to your PR. | ||
|
|
||
| ## Commit & Pull Request Guidelines | ||
| History favors concise, lower-case subjects (`add retry button logic`). Keep messages imperative, under ~72 characters, and group related CSS/JS changes in a single commit where practical. PRs should include: (1) a short summary, (2) manual test notes listing the commands above, (3) before/after visuals for UI tweaks, and (4) linked issues or TODO references. Request reviewers aligned with the change (UI polish, gameplay logic, asset updates) to speed feedback. For larger refactors, note any follow-up tasks (e.g., migrating from inline `onclick` attributes) so the next contributor can pick them up easily. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Docs out of sync with code (bridge is disabled).
AGENTS.md says battleship.js wires controllers and exposes a global bridge, but the file has all wiring commented out in this PR. Either re-enable the bridge (preferred) or update this section to reflect current state.
I'll verify the review comment by checking both AGENTS.md and battleship.js to confirm whether there's a documentation-code mismatch.
🏁 Script executed:
Length of output: 3383
🏁 Script executed:
Length of output: 1638
🏁 Script executed:
Length of output: 462
🏁 Script executed:
Length of output: 20632
🏁 Script executed:
Length of output: 1408
🏁 Script executed:
Length of output: 401
🏁 Script executed:
Length of output: 55
🏁 Script executed:
Length of output: 1641
🏁 Script executed:
Length of output: 3409
🏁 Script executed:
Length of output: 53
🏁 Script executed:
Length of output: 140
🏁 Script executed:
Length of output: 183
🏁 Script executed:
Length of output: 5074
🏁 Script executed:
Length of output: 131
Re-enable the global bridge in battleship.js; it is actively required by index.html's onclick handlers.
AGENTS.md correctly documents that battleship.js exposes a global bridge for
onclickhandlers. index.html relies on this—it callsturnVertical(),turnHorizontal(),selectShip(),clickStartGame(),clickTarget(), andclickCpuBoard()from inline event attributes. All that wiring is currently commented out in battleship.js, so these functions are undefined at runtime and the game will not work. The PR comment itself ("does this need to commented back in to work?") signals uncertainty. Either uncomment the bridge code (preferred) or refactor index.html to remove inline onclick handlers and use event listeners instead.🤖 Prompt for AI Agents