Add classic edition Super Mario Bros 1-1 game - #22
naoto714714 wants to merge 1 commit into
Conversation
ウォークスルーこのプルリクエストは、ブラウザゲーム集に「Super Mario Bros 1-1 Classic Edition」という新作ゲームを追加します。HTML5 Canvas、Vanilla JavaScript、CSS3を使用した完全なゲーム実装で、メインリポジトリのドキュメントとインデックスを更新し、専用ディレクトリに12個のJavaScriptモジュール、スタイルシート、HTMLファイルを含みます。 変更内容
シーケンス図sequenceDiagram
participant User as ユーザー
participant Main as main.js
participant Game as Game
participant Input as InputManager
participant Level as Level
participant Player as Player
participant Physics as Physics
participant Collision as 衝突処理
participant Audio as AudioManager
participant Camera as Camera
participant Render as レンダリング
User->>Main: ページ読み込み
activate Main
Main->>Game: new Game(canvas)
activate Game
Game->>Game: setupCanvas()
Game->>Level: new Level()
activate Level
Level->>Level: generateLevel()
deactivate Level
Game->>Player: new Player()
Game->>Input: new InputManager()
Game->>Camera: new Camera()
Game->>Audio: new AudioManager()
deactivate Game
deactivate Main
User->>Game: Space キー
activate Game
Game->>Game: startGame()
Game->>Game: initializeGame()
Game->>Audio: enableAudio()
Game->>Game: gameLoop()
loop ゲームループ
Game->>Input: update()
Game->>Game: handleInput()
Game->>Player: setInput(...)
Game->>Player: update(deltaTime)
activate Player
Player->>Player: updateMovement()
Player->>Player: updateAnimation()
deactivate Player
Game->>Game: updatePhysics()
activate Physics
Physics->>Physics: applyGravity(Player)
Physics->>Physics: applyFriction(Player)
Physics->>Physics: updatePosition(Player)
Physics->>Collision: resolveCollisionWithTiles()
activate Collision
Collision-->>Physics: 衝突結果
deactivate Collision
deactivate Physics
Game->>Game: handleCollisions()
Game->>Collision: checkPlayerBlockCollisions()
Game->>Collision: checkPlayerEnemyCollisions()
Game->>Collision: checkPlayerItemCollisions()
Game->>Level: update(deltaTime)
activate Level
Level->>Level: 敵/アイテム更新
deactivate Level
Game->>Camera: update(Player)
Game->>Audio: playSound(...)
Game->>Game: render()
activate Render
Render->>Level: render(ctx, camera)
Render->>Player: render(ctx, camera)
deactivate Render
break ゲームオーバー
Game->>Game: gameOver()
end
end
deactivate Game
推定コードレビュー工数🎯 4 (複雑) | ⏱️ 約60-75分 特に注意が必要な領域:
関連する可能性のあるプルリクエスト
詩
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (23)
super-mario-bros-1-1-classic/js/camera.js (1)
38-84: 右方向スクロール時のデッドゾーン処理ロジックの意図確認 & シェイク処理について
Line 43–47 で水平デッドゾーンを計算したあと、Line 57–59 で「
velocityX > 0のとき常にthis.targetX = target.x - this.deadZone.left;」と再代入しているため、右移動中はデッドゾーン判定の結果がほぼ上書きされます。
- 「マリオ風に“右方向のみスクロール”させたい」意図ならありえますが、プレイヤーがデッドゾーン中央付近にいても常に
deadZone.leftに合わせようとするため、カメラがやや前のめりに追従する挙動になります。- もし「デッドゾーンを越えたときだけ追従」させたいのであれば、
velocityX > 0条件をデッドゾーン境界と組み合わせる（例:velocityX > 0 && playerScreenX > this.deadZone.right）など、条件をもう少し絞ると分かりやすくなります。Line 81–84 の
clampToBoundsでは、this.x + this.shake.offsetX/this.y + this.shake.offsetYをそのままthis.x/this.yに書き戻しているため、「ベース位置＋揺れ」ではなく「揺れ込みの位置」を状態として保持する形になっています。
- 機能的には問題にならない可能性も高いですが、一般的には「内部状態としての camera.x/y」と「描画時の揺れオフセット」を分離しておく方が、将来の調整やデバッグがしやすいです（例:
getRenderPosition()で揺れを加算）。挙動が狙い通りか一度プレイして確認しておくと安心です。
super-mario-bros-1-1-classic/js/input.js (1)
47-193: キーコードリテラルとKEYS定数の統一についてこの
InputManagerはevent.codeベースでキー状態を管理しており、Line 49, 97–113, 141–157 などで'ArrowLeft','ArrowRight','Space','KeyX','KeyP'といったリテラルを直接使っています。一方でconstants.jsにはKEYSオブジェクトがあり、特にKEYS.JUMPが' '（スペース文字）になっているため、将来KEYSを参照するコードと食い違う可能性があります。
KEYS側を'Space'などKeyboardEvent.codeに合わせた値に修正したうえで、InputManager内の文字列もKEYS.JUMP/KEYS.RUNなどの定数経由に寄せると、
- キー割り当て変更時の修正漏れを防げる
- ゲーム内でキー定義の単一ソースを保てる
というメリットがあります。ゲーム全体のキー定義をどこで正とするか、一度決めておくとメンテしやすいです。
super-mario-bros-1-1-classic/README.md (1)
55-75: コードブロックに言語指定を付けると markdownlint が通りますLine 55 のディレクトリ構成のコードブロックが ````` のみで開始されており、
markdownlint(MD040: fenced-code-language) の警告対象になっています。例えば次のように言語を付けておくと、Lint も静かになりますし閲覧時のシンタックスハイライトも安定します。
-``` +```text super-mario-bros-1-1-classic/ ├── index.html # メインHTML ...super-mario-bros-1-1-classic/js/main.js (1)
2-85: グローバル汚染とガイドラインとの整合性について初期化フロー自体は分かりやすくて良いのですが、プロジェクトルール（
*/js/**/*.js : グローバル変数は使用しない）との観点で気になる点がいくつかあります。
- Line 56–57:
window.DEBUGを直接立てています。- Line 67:
window.game = game;でゲームインスタンスをグローバルに公開しています。- このファイルに限りませんが、
SpriteLoaderやGameもグローバル名前空間上のクラスとして利用されています。このゲームは単独ページで動くので即問題になるわけではありませんが、「ブラウザ全体で複数ゲームを扱う」というリポジトリ方針を考えると、将来的には以下のような形に寄せていくと安全です。
- IIFE や名前空間オブジェクトに閉じ込める:
window.MarioClassic = { Game, SpriteLoader, ... }など。- もしくは
<script type="module">化して、import/exportベースで依存関係を表現する。- デバッグ用の
window.game/window.DEBUGも、可能であれば「デバッグ時のみ有効」にするラッパー（例:if (window.location.hash === '#debug') window.MarioClassicDebug = { game };）に寄せる。現状でも動作上の問題はなさそうですが、リポジトリ全体のルールに揃えるという意味で、余裕のあるタイミングで見直しておくと良さそうです。
super-mario-bros-1-1-classic/js/constants.js (1)
1-147: グローバル定数の定義方針についてこのファイルの
GAME_CONSTANTS、DIRECTION、BLOCK_TYPES、ENEMY_TYPES、ITEM_TYPES、KEYSはすべてトップレベルconstとして定義されており、<script>直読み構成ではグローバル名前空間に載ります。リポジトリのルール（*/js/**/*.js : ゲーム間で名前衝突を防ぐため、グローバル変数は使用しない）に照らすと、将来的には IIFE や名前空間オブジェクト、あるいは ES Modules (type="module") への移行を検討しておくと安心です。補足:
KEYS定数は定義されていますが、実装では使用されておらず、InputManager は直接event.code文字列（'Space'、'ArrowLeft'など）を使用しています。super-mario-bros-1-1-classic/js/utils.js (2)
1-2: グローバル変数の使用はコーディングガイドラインに違反しています。
Utilsがグローバル定数として宣言されており、ゲーム間での名前衝突を引き起こす可能性があります。モジュールパターンまたはIIFEでカプセル化することを推奨します。Based on coding guidelines and learnings.-// ユーティリティ関数 -const Utils = { +// ユーティリティ関数 +const MarioClassicUtils = (() => { + const Utils = { // 矩形の衝突判定 checkCollision(rect1, rect2) {ファイル末尾を以下のように変更:
getAnimationFrame(frameCount, frameRate, totalFrames) { return Math.floor(frameCount / frameRate) % totalFrames; }, -}; + }; + return Utils; +})();または、ES6モジュール形式への移行を検討してください。
111-129: deepClone関数の冗長なチェックとエッジケース処理Line 122の
typeof obj === 'object'チェックは、Line 113の条件で非オブジェクトは既にリターンされているため冗長です。また、Map、Set、RegExpなどの特殊オブジェクトは正しくクローンされません。deepClone(obj) { if (obj === null || typeof obj !== 'object') { return obj; } if (obj instanceof Date) { return new Date(obj.getTime()); } if (obj instanceof Array) { return obj.map((item) => this.deepClone(item)); } - if (typeof obj === 'object') { - const clonedObj = {}; - for (const key in obj) { - clonedObj[key] = this.deepClone(obj[key]); - } - return clonedObj; + const clonedObj = {}; + for (const key in obj) { + if (Object.prototype.hasOwnProperty.call(obj, key)) { + clonedObj[key] = this.deepClone(obj[key]); + } } + return clonedObj; },
hasOwnPropertyチェックを追加することで、継承されたプロパティのコピーを防ぎます。super-mario-bros-1-1-classic/js/enemies.js (2)
1-2: グローバルクラス宣言はコーディングガイドラインに違反しています。
Enemy、Goomba、KoopaTroopaがグローバルスコープで宣言されています。名前衝突を防ぐため、名前空間またはモジュールパターンでカプセル化してください。Based on coding guidelines.
182-183: マジックナンバーを定数に抽出することを推奨します。甲羅の速度閾値
0.1と蹴り速度8が複数箇所で使用されています。GAME_CONSTANTSに追加するか、クラス定数として定義することで保守性が向上します。// GAME_CONSTANTSに追加 KOOPA: { // ...existing SHELL_VELOCITY_THRESHOLD: 0.1, SHELL_KICK_SPEED: 8, },super-mario-bros-1-1-classic/css/style.css (2)
84-90: 重複したimage-renderingプロパティについて（静的解析の警告）これはクロスブラウザ対応のフォールバックパターンとして意図的に記述されていると理解しますが、
image-rendering: pixelatedは標準プロパティとして広くサポートされているため、ベンダープレフィックス版のみをフォールバックとして残すことを検討してください。#gameCanvas { display: block; margin-top: 40px; - image-rendering: pixelated; image-rendering: -moz-crisp-edges; image-rendering: crisp-edges; + image-rendering: pixelated; }最後に
pixelatedを配置することで、対応ブラウザでは標準プロパティが優先されます。同様にLine 157-161のcanvasルールも修正してください。
163-168:transform: scaleはピクセルアートのレンダリング品質に影響する可能性があります。モバイル対応に
transform: scaleを使用していますが、ピクセルパーフェクトなゲームでは拡大縮小時にぼやける可能性があります。JavaScriptでキャンバスサイズを動的に調整する方法も検討してください。super-mario-bros-1-1-classic/js/physics.js (3)
1-2: グローバル変数の使用はコーディングガイドラインに違反しています。
Physicsがグローバル定数として宣言されています。他のモジュール（Utils、GAME_CONSTANTS）と同様に、名前空間またはモジュールパターンでカプセル化してください。Based on coding guidelines.
27-35:Utils.checkCollisionとの重複コードこの実装は
Utils.checkCollisionと同一です。DRY原則に従い、既存の関数を再利用することを推奨します。// 矩形同士の衝突検出 checkRectCollision(rect1, rect2) { - return ( - rect1.x < rect2.x + rect2.width && - rect1.x + rect1.width > rect2.x && - rect1.y < rect2.y + rect2.height && - rect1.y + rect1.height > rect2.y - ); + return Utils.checkCollision(rect1, rect2); },
115-122: バウンス減衰係数のマジックナンバー
0.6の減衰係数がハードコードされています。GAME_CONSTANTSに追加することで調整が容易になります。// GAME_CONSTANTSに追加 BOUNCE_DAMPING: 0.6,super-mario-bros-1-1-classic/js/spriteLoader.js (1)
1-3: グローバル変数の使用を避けてください。
SpriteLoaderがグローバルスコープで定義されています。コーディングガイドラインに従い、IIFE（即時実行関数式）やモジュールパターンを使用して名前空間を保護することを推奨します。-// スプライトローダー（ピクセルアート生成） -const SpriteLoader = { - sprites: {}, +// スプライトローダー（ピクセルアート生成） +(function(global) { + const SpriteLoader = { + sprites: {}, + // ... 既存のコード ... + }; + global.SpriteLoader = SpriteLoader; +})(window);または ES モジュールの使用を検討してください。
Based on coding guidelines, グローバル変数は使用しない。
super-mario-bros-1-1-classic/js/entities.js (2)
1-2: グローバルスコープでのクラス定義。
Entity、Block、Itemクラスがグローバルスコープで定義されています。他のゲームとの名前衝突を防ぐため、モジュールパターンまたは ES モジュールでカプセル化することを検討してください。Based on coding guidelines, グローバル変数は使用しない。
21-29:deltaTimeパラメータが未使用。
update(deltaTime)メソッドでdeltaTimeを受け取っていますが、内部で使用されていません。フレームレート独立の更新が意図されている場合は、アニメーションや物理処理で活用を検討してください。super-mario-bros-1-1-classic/js/level.js (2)
1-2: グローバルスコープでのクラス定義。
Levelクラスがグローバルスコープで定義されています。モジュールパターンの使用を検討してください。Based on coding guidelines, グローバル変数は使用しない。
406-422:getEntitiesInRangeの重複呼び出し。同じパラメータで
getEntitiesInRangeを3回呼び出しています。1回の呼び出しで結果をキャッシュすることでパフォーマンスを改善できます：- // ブロックの描画 - const visibleBlocks = this.getEntitiesInRange(camera.x, GAME_CONSTANTS.CANVAS_WIDTH).blocks; - visibleBlocks.forEach((block) => { - block.render(ctx, camera); - }); - - // 敵の描画 - const visibleEnemies = this.getEntitiesInRange(camera.x, GAME_CONSTANTS.CANVAS_WIDTH).enemies; - visibleEnemies.forEach((enemy) => { - enemy.render(ctx, camera); - }); - - // アイテムの描画 - const visibleItems = this.getEntitiesInRange(camera.x, GAME_CONSTANTS.CANVAS_WIDTH).items; - visibleItems.forEach((item) => { - item.render(ctx, camera); - }); + // 可視エンティティを一度に取得 + const { blocks, enemies, items } = this.getEntitiesInRange(camera.x, GAME_CONSTANTS.CANVAS_WIDTH); + + blocks.forEach((block) => block.render(ctx, camera)); + enemies.forEach((enemy) => enemy.render(ctx, camera)); + items.forEach((item) => item.render(ctx, camera));super-mario-bros-1-1-classic/js/player.js (2)
1-2: グローバルスコープでのクラス定義。
Playerクラスがグローバルスコープで定義されています。モジュールパターンの使用を検討してください。Based on coding guidelines, グローバル変数は使用しない。
164-167: 大きいマリオのスプライトが未実装。BIG/FIRE 状態で小さいマリオのスプライトを使用しています。将来的にスプライトを追加する際は、この分岐を更新する必要があります。
この機能の実装を手伝いましょうか？
super-mario-bros-1-1-classic/js/game.js (2)
1-2: グローバルスコープでのクラス定義。
Gameクラスがグローバルスコープで定義されています。モジュールパターンの使用を検討してください。Based on coding guidelines, グローバル変数は使用しない。
197-201:instanceofチェックによる密結合。
enemy instanceof Goombaのチェックは密結合を生みます。代わりにメソッドの存在チェックやポリモーフィズムを使用することを検討してください：- if (enemy instanceof Goomba) { - enemy.checkDirectionChange(solidBlocks); - } + if (typeof enemy.checkDirectionChange === 'function') { + enemy.checkDirectionChange(solidBlocks); + }これにより、新しい敵タイプを追加する際の変更が容易になります。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
README.md(1 hunks)index.html(1 hunks)super-mario-bros-1-1-classic/README.md(1 hunks)super-mario-bros-1-1-classic/css/style.css(1 hunks)super-mario-bros-1-1-classic/index.html(1 hunks)super-mario-bros-1-1-classic/js/audio.js(1 hunks)super-mario-bros-1-1-classic/js/camera.js(1 hunks)super-mario-bros-1-1-classic/js/constants.js(1 hunks)super-mario-bros-1-1-classic/js/enemies.js(1 hunks)super-mario-bros-1-1-classic/js/entities.js(1 hunks)super-mario-bros-1-1-classic/js/game.js(1 hunks)super-mario-bros-1-1-classic/js/input.js(1 hunks)super-mario-bros-1-1-classic/js/level.js(1 hunks)super-mario-bros-1-1-classic/js/main.js(1 hunks)super-mario-bros-1-1-classic/js/physics.js(1 hunks)super-mario-bros-1-1-classic/js/player.js(1 hunks)super-mario-bros-1-1-classic/js/spriteLoader.js(1 hunks)super-mario-bros-1-1-classic/js/utils.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
*/js/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/about-this-project.mdc)
ゲーム間で名前衝突を防ぐため、グローバル変数は使用しない
Files:
super-mario-bros-1-1-classic/js/camera.jssuper-mario-bros-1-1-classic/js/main.jssuper-mario-bros-1-1-classic/js/enemies.jssuper-mario-bros-1-1-classic/js/physics.jssuper-mario-bros-1-1-classic/js/utils.jssuper-mario-bros-1-1-classic/js/audio.jssuper-mario-bros-1-1-classic/js/player.jssuper-mario-bros-1-1-classic/js/input.jssuper-mario-bros-1-1-classic/js/level.jssuper-mario-bros-1-1-classic/js/entities.jssuper-mario-bros-1-1-classic/js/spriteLoader.jssuper-mario-bros-1-1-classic/js/constants.jssuper-mario-bros-1-1-classic/js/game.js
*/index.html
📄 CodeRabbit inference engine (.cursor/rules/about-this-project.mdc)
*/index.html: 動的コードが必要な場合も、入口は index.html に集約 (モジュール分割は js/ 内)
外部ライブラリは CDN 参照を推奨 (ローカル配置も可)
index.html にゲームロジックを実装 (必要に応じて js/・css/ を参照)各ゲームは
index.htmlから開始する（エントリーポイント）
Files:
super-mario-bros-1-1-classic/index.html
**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
外部ライブラリはCDN参照を推奨する
Files:
super-mario-bros-1-1-classic/index.htmlindex.html
README*
📄 CodeRabbit inference engine (.cursor/rules/coding-rule.mdc)
README に成果物の概要を記載
Files:
README.md
🧠 Learnings (6)
📚 Learning: 2025-07-20T03:15:07.280Z
Learnt from: CR
Repo: naoto714714/browser-games PR: 0
File: .cursor/rules/about-this-project.mdc:0-0
Timestamp: 2025-07-20T03:15:07.280Z
Learning: Applies to */index.html : index.html にゲームロジックを実装 (必要に応じて js/・css/ を参照)
Applied to files:
super-mario-bros-1-1-classic/index.htmlsuper-mario-bros-1-1-classic/README.mdsuper-mario-bros-1-1-classic/js/main.jsindex.htmlsuper-mario-bros-1-1-classic/css/style.csssuper-mario-bros-1-1-classic/js/game.js
📚 Learning: 2025-07-27T03:49:03.596Z
Learnt from: CR
Repo: naoto714714/browser-games PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T03:49:03.596Z
Learning: Applies to */index.html : 各ゲームは `index.html` から開始する（エントリーポイント）
Applied to files:
super-mario-bros-1-1-classic/index.htmlsuper-mario-bros-1-1-classic/README.mdsuper-mario-bros-1-1-classic/js/main.jsindex.htmlsuper-mario-bros-1-1-classic/css/style.csssuper-mario-bros-1-1-classic/js/game.js
📚 Learning: 2025-07-20T03:15:07.280Z
Learnt from: CR
Repo: naoto714714/browser-games PR: 0
File: .cursor/rules/about-this-project.mdc:0-0
Timestamp: 2025-07-20T03:15:07.280Z
Learning: Applies to */index.html : 動的コードが必要な場合も、入口は index.html に集約 (モジュール分割は js/ 内)
Applied to files:
super-mario-bros-1-1-classic/index.htmlsuper-mario-bros-1-1-classic/js/main.js
📚 Learning: 2025-07-20T03:15:07.280Z
Learnt from: CR
Repo: naoto714714/browser-games PR: 0
File: .cursor/rules/about-this-project.mdc:0-0
Timestamp: 2025-07-20T03:15:07.280Z
Learning: 新規ゲーム追加時は <ゲーム名>/ ディレクトリを作成し、index.html とREADME.mdを配置
Applied to files:
super-mario-bros-1-1-classic/index.htmlsuper-mario-bros-1-1-classic/js/main.jsindex.html
📚 Learning: 2025-07-20T03:15:07.280Z
Learnt from: CR
Repo: naoto714714/browser-games PR: 0
File: .cursor/rules/about-this-project.mdc:0-0
Timestamp: 2025-07-20T03:15:07.280Z
Learning: Applies to */js/**/*.js : ゲーム間で名前衝突を防ぐため、グローバル変数は使用しない
Applied to files:
super-mario-bros-1-1-classic/js/main.jssuper-mario-bros-1-1-classic/js/utils.jssuper-mario-bros-1-1-classic/js/constants.js
📚 Learning: 2025-07-20T03:15:07.280Z
Learnt from: CR
Repo: naoto714714/browser-games PR: 0
File: .cursor/rules/about-this-project.mdc:0-0
Timestamp: 2025-07-20T03:15:07.280Z
Learning: Applies to common/js/**/*.js : 共通ユーティリティが必要な場合は /common/js/ などに配置し、各ゲームは相対パスで参照
Applied to files:
super-mario-bros-1-1-classic/js/utils.js
🧬 Code graph analysis (10)
super-mario-bros-1-1-classic/js/enemies.js (2)
super-mario-bros-1-1-classic/js/constants.js (3)
DIRECTION(108-111)GAME_CONSTANTS(2-105)ENEMY_TYPES(127-130)super-mario-bros-1-1-classic/js/physics.js (1)
Physics(2-198)
super-mario-bros-1-1-classic/js/physics.js (2)
super-mario-bros-1-1-classic/js/constants.js (1)
GAME_CONSTANTS(2-105)super-mario-bros-1-1-classic/js/utils.js (1)
Utils(2-135)
super-mario-bros-1-1-classic/js/utils.js (1)
super-mario-bros-1-1-classic/js/constants.js (1)
GAME_CONSTANTS(2-105)
super-mario-bros-1-1-classic/js/audio.js (1)
super-mario-bros-1-1-classic/js/utils.js (1)
Utils(2-135)
super-mario-bros-1-1-classic/js/player.js (1)
super-mario-bros-1-1-classic/js/constants.js (3)
GAME_CONSTANTS(2-105)DIRECTION(108-111)ITEM_TYPES(133-138)
super-mario-bros-1-1-classic/js/input.js (1)
bird-and-beans/js/input.js (1)
InputManager(1-53)
super-mario-bros-1-1-classic/js/level.js (2)
super-mario-bros-1-1-classic/js/constants.js (4)
GAME_CONSTANTS(2-105)BLOCK_TYPES(114-124)ENEMY_TYPES(127-130)ITEM_TYPES(133-138)super-mario-bros-1-1-classic/js/spriteLoader.js (3)
x(30-30)y(29-29)brick(157-174)
super-mario-bros-1-1-classic/js/entities.js (3)
super-mario-bros-1-1-classic/js/spriteLoader.js (3)
x(30-30)y(29-29)SpriteLoader(2-255)super-mario-bros-1-1-classic/js/constants.js (4)
DIRECTION(108-111)GAME_CONSTANTS(2-105)BLOCK_TYPES(114-124)ITEM_TYPES(133-138)super-mario-bros-1-1-classic/js/physics.js (1)
Physics(2-198)
super-mario-bros-1-1-classic/js/spriteLoader.js (1)
super-mario-bros-1-1-classic/js/main.js (1)
canvas(6-6)
super-mario-bros-1-1-classic/js/game.js (7)
bird-and-beans/js/game.js (1)
Game(10-156)super-mario-bros-1-1-classic/js/main.js (1)
canvas(6-6)super-mario-bros-1-1-classic/js/spriteLoader.js (1)
canvas(21-21)super-mario-bros-1-1-classic/js/constants.js (3)
GAME_CONSTANTS(2-105)ITEM_TYPES(133-138)BLOCK_TYPES(114-124)bird-and-beans/js/audio.js (1)
AudioManager(3-68)super-mario-bros-1-1-classic/js/physics.js (1)
Physics(2-198)super-mario-bros-1-1-classic/js/utils.js (1)
Utils(2-135)
🪛 Biome (2.1.2)
super-mario-bros-1-1-classic/css/style.css
[error] 88-88: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
image-rendering is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 159-160: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
image-rendering is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🪛 markdownlint-cli2 (0.18.1)
super-mario-bros-1-1-classic/README.md
55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (12)
README.md (1)
7-10: クラシック版エントリ追加は問題ありません既存セクションとスタイルが揃っており、概要テキストとリンク先 (
/super-mario-bros-1-1-classic/) も妥当です。README に新しい成果物の概要がきちんと追記されていて良いと思います。index.html (1)
98-102: ゲームカード追加の構成は良さそうです
- 既存カードと同じマークアップ・クラス構成で、デザイン統一が保たれています。
- リンク先
./super-mario-bros-1-1-classic/も新ディレクトリ構成と一致しています。トップレベルの
index.htmlからクラシック版に遷移できるようになっており、プロジェクトのルールにも沿っていると思います。super-mario-bros-1-1-classic/index.html (1)
1-48: ゲーム用index.htmlの構成はルールに沿っています
#gameCanvas,#startScreen,#gameOverScreenなど、main.js／Gameから参照される要素がすべて定義されています。- スクリプトも
constants.js→utils.js→ … →main.jsの順で読み込んでおり、依存関係的にも自然です。- 「入口は index.html に集約し、ロジックは js/ に分割」というリポジトリルールにも適合していると思います。
特に問題になる点は見当たりません。
super-mario-bros-1-1-classic/js/enemies.js (2)
63-70: 踏みつけ判定のロジックは妥当ですが、エッジケースに注意。プレイヤーの前フレーム位置を使用した踏みつけ判定は適切です。ただし、高速落下時（
velocityYが大きい場合）に判定が不安定になる可能性があります。必要に応じて、より堅牢な判定方法を検討してください。
96-101: This concern is incorrect —animationTimeris properly initialized.
animationTimeris initialized to0in theEntityclass constructor (super-mario-bros-1-1-classic/js/entities.js), whichGoombainherits throughEnemy. The property will work as expected. No initialization issue exists.super-mario-bros-1-1-classic/js/physics.js (1)
37-95: タイル衝突解決のロジックは適切に実装されています。水平・垂直を分離した衝突解決は標準的なアプローチです。最初の衝突で
breakしているため、高速移動時のトンネリングが発生する可能性がありますが、このゲームの規模では許容範囲です。super-mario-bros-1-1-classic/js/spriteLoader.js (2)
19-61: LGTM!
createCanvasとgetColorの実装は正確で、ピクセルアートのレンダリングに適切です。未知の色コードに対するマゼンタのフォールバックも良いデバッグ手法です。
63-254: LGTM!スプライト作成メソッドは適切に構造化されており、16x16のピクセルデータが正しく定義されています。
getSpriteアクセサも null セーフティを提供しています。super-mario-bros-1-1-classic/js/entities.js (1)
235-247: LGTM!
Itemクラスの実装は適切です。アイテムタイプに応じた動作設定と収集処理が正しく分離されています。super-mario-bros-1-1-classic/js/level.js (1)
26-51: LGTM!地面生成ロジックは正確で、穴の作成も適切に処理されています。タイル座標の計算も正しいです。
super-mario-bros-1-1-classic/js/player.js (1)
101-117: LGTM!ジャンプ実装は適切です。コヨーテタイムとジャンプバッファの併用により、快適な操作感が実現されています。
super-mario-bros-1-1-classic/js/game.js (1)
395-426: LGTM!HUD 更新と描画処理は適切に実装されています。null チェックによる防御的プログラミングと、条件付きデバッグ情報の表示が良いです。
| // 音響管理クラス（Web Audio API使用） | ||
| class AudioManager { | ||
| constructor() { | ||
| this.audioContext = null; | ||
| this.sounds = {}; | ||
| this.musicVolume = 0.5; | ||
| this.sfxVolume = 0.7; | ||
| this.enabled = true; | ||
|
|
||
| this.initAudioContext(); | ||
| this.createSounds(); | ||
| } |
There was a problem hiding this comment.
グローバルクラス宣言とプロパティ初期化の問題
AudioManagerがグローバルスコープで宣言されており、コーディングガイドラインに違反しています。また、musicEnabledプロパティがコンストラクタで初期化されていません。
class AudioManager {
constructor() {
this.audioContext = null;
this.sounds = {};
this.musicVolume = 0.5;
this.sfxVolume = 0.7;
this.enabled = true;
+ this.musicEnabled = false;
this.initAudioContext();
this.createSounds();
}🤖 Prompt for AI Agents
In super-mario-bros-1-1-classic/js/audio.js around lines 1-12, the AudioManager
class is currently declared in the global scope and the constructor omits
initialization of the musicEnabled property; fix this by removing the global
exposure (declare the class as a module-local export — e.g., export the class or
wrap it in a module/namespace so it isn’t placed on window/global) and add an
explicit this.musicEnabled = <boolean> initialization in the constructor (set to
the intended default, e.g., true or false).
| // BGM（簡易版） | ||
| playBGM() { | ||
| if (!this.enabled || !this.audioContext) { | ||
| return; | ||
| } | ||
|
|
||
| // マリオのメインテーマの一部（プロシージャル） | ||
| const melody = [ | ||
| 659, 659, 0, 659, 0, 523, 659, 0, 784, 0, 0, 392, 0, 0, 523, 0, 0, 392, 0, 0, 330, 0, 0, 440, 0, 494, 0, 466, 0, | ||
| 440, 0, 392, 659, 784, 880, 0, 698, 784, 0, 659, 0, 523, 587, 523, | ||
| ]; | ||
|
|
||
| let index = 0; | ||
| const playNote = () => { | ||
| if (index >= melody.length) { | ||
| index = 0; // ループ | ||
| } | ||
|
|
||
| const frequency = melody[index]; | ||
| if (frequency > 0) { | ||
| this.createTone(frequency, 0.3, 'square')(); | ||
| } | ||
|
|
||
| index++; | ||
| setTimeout(playNote, 200); | ||
| }; | ||
|
|
||
| if (this.musicEnabled) { | ||
| playNote(); | ||
| } | ||
| } | ||
|
|
||
| // BGM停止 | ||
| stopBGM() { | ||
| this.musicEnabled = false; | ||
| } |
There was a problem hiding this comment.
BGMループが正しく停止できず、musicEnabledチェックに問題があります。
- Line 207で
this.musicEnabledをチェックしていますが、コンストラクタで初期化されていないため常にundefined（falsy）となり、BGMが再生されません。 setTimeoutの参照を保持していないため、stopBGM()を呼んでもタイマーチェーンを停止できません。
+ // BGMタイマーの参照
+ bgmTimeoutId = null;
// BGM（簡易版）
playBGM() {
if (!this.enabled || !this.audioContext) {
return;
}
+ this.musicEnabled = true;
+
// マリオのメインテーマの一部（プロシージャル）
const melody = [
659, 659, 0, 659, 0, 523, 659, 0, 784, 0, 0, 392, 0, 0, 523, 0, 0, 392, 0, 0, 330, 0, 0, 440, 0, 494, 0, 466, 0,
440, 0, 392, 659, 784, 880, 0, 698, 784, 0, 659, 0, 523, 587, 523,
];
let index = 0;
const playNote = () => {
+ if (!this.musicEnabled) {
+ return;
+ }
if (index >= melody.length) {
index = 0; // ループ
}
const frequency = melody[index];
if (frequency > 0) {
this.createTone(frequency, 0.3, 'square')();
}
index++;
- setTimeout(playNote, 200);
+ this.bgmTimeoutId = setTimeout(playNote, 200);
};
- if (this.musicEnabled) {
- playNote();
- }
+ playNote();
}
// BGM停止
stopBGM() {
this.musicEnabled = false;
+ if (this.bgmTimeoutId) {
+ clearTimeout(this.bgmTimeoutId);
+ this.bgmTimeoutId = null;
+ }
}📝 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.
| // BGM（簡易版） | |
| playBGM() { | |
| if (!this.enabled || !this.audioContext) { | |
| return; | |
| } | |
| // マリオのメインテーマの一部（プロシージャル） | |
| const melody = [ | |
| 659, 659, 0, 659, 0, 523, 659, 0, 784, 0, 0, 392, 0, 0, 523, 0, 0, 392, 0, 0, 330, 0, 0, 440, 0, 494, 0, 466, 0, | |
| 440, 0, 392, 659, 784, 880, 0, 698, 784, 0, 659, 0, 523, 587, 523, | |
| ]; | |
| let index = 0; | |
| const playNote = () => { | |
| if (index >= melody.length) { | |
| index = 0; // ループ | |
| } | |
| const frequency = melody[index]; | |
| if (frequency > 0) { | |
| this.createTone(frequency, 0.3, 'square')(); | |
| } | |
| index++; | |
| setTimeout(playNote, 200); | |
| }; | |
| if (this.musicEnabled) { | |
| playNote(); | |
| } | |
| } | |
| // BGM停止 | |
| stopBGM() { | |
| this.musicEnabled = false; | |
| } | |
| // BGMタイマーの参照 | |
| bgmTimeoutId = null; | |
| // BGM（簡易版） | |
| playBGM() { | |
| if (!this.enabled || !this.audioContext) { | |
| return; | |
| } | |
| this.musicEnabled = true; | |
| // マリオのメインテーマの一部（プロシージャル） | |
| const melody = [ | |
| 659, 659, 0, 659, 0, 523, 659, 0, 784, 0, 0, 392, 0, 0, 523, 0, 0, 392, 0, 0, 330, 0, 0, 440, 0, 494, 0, 466, 0, | |
| 440, 0, 392, 659, 784, 880, 0, 698, 784, 0, 659, 0, 523, 587, 523, | |
| ]; | |
| let index = 0; | |
| const playNote = () => { | |
| if (!this.musicEnabled) { | |
| return; | |
| } | |
| if (index >= melody.length) { | |
| index = 0; // ループ | |
| } | |
| const frequency = melody[index]; | |
| if (frequency > 0) { | |
| this.createTone(frequency, 0.3, 'square')(); | |
| } | |
| index++; | |
| this.bgmTimeoutId = setTimeout(playNote, 200); | |
| }; | |
| playNote(); | |
| } | |
| // BGM停止 | |
| stopBGM() { | |
| this.musicEnabled = false; | |
| if (this.bgmTimeoutId) { | |
| clearTimeout(this.bgmTimeoutId); | |
| this.bgmTimeoutId = null; | |
| } | |
| } |
| // ブロックが叩かれた時の処理 | ||
| onHit(player) { | ||
| if (this.hit) { | ||
| return null; | ||
| } | ||
|
|
||
| this.hit = true; | ||
| this.bumpAnimation = 10; // バンプアニメーション開始 | ||
|
|
||
| if (this.type === BLOCK_TYPES.BRICK) { | ||
| if (player.powerState === GAME_CONSTANTS.PLAYER_STATE.SMALL) { | ||
| // 小さいマリオは壊せない | ||
| return null; | ||
| } else { | ||
| // 大きいマリオは壊せる | ||
| this.broken = true; | ||
| this.destroy(); | ||
| return { type: 'break', points: 50 }; | ||
| } | ||
| } else if (this.type === BLOCK_TYPES.QUESTION) { | ||
| this.spriteKey = 'brick'; | ||
| if (this.item) { | ||
| const itemResult = this.spawnItem(); | ||
| return itemResult; | ||
| } else { | ||
| // コインを出す | ||
| return { type: 'coin', points: GAME_CONSTANTS.ITEM.COIN.POINTS }; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
小さいマリオがレンガブロックを叩いた時のバグ。
Line 159 で this.hit = true を設定した後、Line 163-165 で小さいマリオはブロックを壊せないため null を返しています。しかし、hit フラグが既に true になっているため、小さいマリオは同じブロックを再度叩けなくなります。
レンガブロックの場合は、壊れた時のみ hit を true にすべきです：
onHit(player) {
if (this.hit) {
return null;
}
- this.hit = true;
this.bumpAnimation = 10; // バンプアニメーション開始
if (this.type === BLOCK_TYPES.BRICK) {
if (player.powerState === GAME_CONSTANTS.PLAYER_STATE.SMALL) {
// 小さいマリオは壊せない
return null;
} else {
// 大きいマリオは壊せる
+ this.hit = true;
this.broken = true;
this.destroy();
return { type: 'break', points: 50 };
}
} else if (this.type === BLOCK_TYPES.QUESTION) {
+ this.hit = true;
this.spriteKey = 'brick';🤖 Prompt for AI Agents
In super-mario-bros-1-1-classic/js/entities.js around lines 153-184, the code
currently sets this.hit = true unconditionally which prevents small Mario from
re-hitting a brick; change the logic so this.hit is only set when the block has
been consumed/processed: keep this.bumpAnimation = 10 as-is, but remove the
unconditional this.hit = true and instead set this.hit = true inside the BRICK
branch only when the brick is actually broken, and set this.hit = true inside
the QUESTION branch when an item/coin is spawned (after spawnItem or coin result
is determined) so blocks that should remain hittable by small Mario stay
hittable.
| // ゲームループ | ||
| gameLoop() { | ||
| if (this.state !== GAME_CONSTANTS.GAME_STATE.PLAYING) { | ||
| return; | ||
| } | ||
|
|
||
| const currentTime = performance.now(); | ||
| this.deltaTime = (currentTime - this.lastFrameTime) / 1000; | ||
| this.lastFrameTime = currentTime; |
There was a problem hiding this comment.
初回フレームの deltaTime が不正確。
lastFrameTime が 0 で初期化されているため、最初のフレームで deltaTime がページ読み込みからの経過時間になり、物理演算に異常が発生する可能性があります。
// ゲーム開始
startGame() {
this.state = GAME_CONSTANTS.GAME_STATE.PLAYING;
document.getElementById('startScreen').classList.add('hidden');
// ゲームオブジェクトを初期化
this.initializeGame();
// 音響を有効化
this.audioManager.enableAudio();
+ // フレーム時間を初期化
+ this.lastFrameTime = performance.now();
// ゲームループ開始
this.gameLoop();
}📝 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.
| // ゲームループ | |
| gameLoop() { | |
| if (this.state !== GAME_CONSTANTS.GAME_STATE.PLAYING) { | |
| return; | |
| } | |
| const currentTime = performance.now(); | |
| this.deltaTime = (currentTime - this.lastFrameTime) / 1000; | |
| this.lastFrameTime = currentTime; | |
| // ゲーム開始 | |
| startGame() { | |
| this.state = GAME_CONSTANTS.GAME_STATE.PLAYING; | |
| document.getElementById('startScreen').classList.add('hidden'); | |
| // ゲームオブジェクトを初期化 | |
| this.initializeGame(); | |
| // 音響を有効化 | |
| this.audioManager.enableAudio(); | |
| // フレーム時間を初期化 | |
| this.lastFrameTime = performance.now(); | |
| // ゲームループ開始 | |
| this.gameLoop(); | |
| } |
🤖 Prompt for AI Agents
In super-mario-bros-1-1-classic/js/game.js around lines 86 to 94, the
first-frame deltaTime is incorrect because lastFrameTime is initialized to 0;
fix by initializing lastFrameTime to performance.now() when the game enters
PLAYING (or, in gameLoop, detect lastFrameTime === 0 and set deltaTime = 0 and
lastFrameTime = performance.now() before using it) so the first frame uses a
sensible delta (0) instead of elapsed time since page load.
| // ゲーム開始（スタート画面から） | ||
| if (this.state === GAME_CONSTANTS.GAME_STATE.START && input.jumpJustPressed) { | ||
| this.startGame(); | ||
| return; | ||
| } | ||
|
|
||
| // ゲームオーバー画面からリスタート | ||
| if (this.state === GAME_CONSTANTS.GAME_STATE.GAME_OVER && input.jumpJustPressed) { | ||
| this.restartGame(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for event listeners and input handling in the super-mario game
rg -n "addEventListener|keydown|keypress|click" super-mario-bros-1-1-classic/js/Repository: naoto714714/browser-games
Length of output: 1499
🏁 Script executed:
# Get the full game.js file to understand the control flow
wc -l super-mario-bros-1-1-classic/js/game.jsRepository: naoto714714/browser-games
Length of output: 112
🏁 Script executed:
# Check for main.js or input manager
find super-mario-bros-1-1-classic -name "*.js" -type fRepository: naoto714714/browser-games
Length of output: 619
🏁 Script executed:
# Read game.js around the gameLoop area to understand the control flow
sed -n '80,100p' super-mario-bros-1-1-classic/js/game.jsRepository: naoto714714/browser-games
Length of output: 481
🏁 Script executed:
# Read game.js around lines 125-135 to see handleInput in context
sed -n '115,145p' super-mario-bros-1-1-classic/js/game.jsRepository: naoto714714/browser-games
Length of output: 786
🏁 Script executed:
# Read main.js to understand the event listener setup
cat -n super-mario-bros-1-1-classic/js/main.jsRepository: naoto714714/browser-games
Length of output: 3006
The handleInput checks for START/GAME_OVER states (lines 126-133) are unreachable dead code.
While input handling during START and GAME_OVER states functions correctly—main.js has separate event listeners (lines 28-42, 45-53) that directly call startGame() and restartGame()—the state checks in handleInput will never execute because gameLoop exits early if state !== PLAYING (lines 88-90).
Remove the unreachable checks for START and GAME_OVER states from handleInput, or restructure to clarify that these states are handled outside of gameLoop.
🤖 Prompt for AI Agents
In super-mario-bros-1-1-classic/js/game.js around lines 125 to 135, the input
checks for START and GAME_OVER are unreachable because gameLoop exits early when
state !== PLAYING; remove the two if blocks that call startGame() and
restartGame() from handleInput, leaving only PLAYING-related input handling, and
add a short comment above handleInput noting that START/GAME_OVER are handled by
main.js event listeners so they are intentionally omitted here.
| // 移動処理の更新 | ||
| updateMovement() { | ||
| // コヨーテタイム（地面から離れた直後でもジャンプ可能） | ||
| if (!this.grounded && this.coyoteTime > 0) { | ||
| this.coyoteTime--; | ||
| } | ||
|
|
||
| // ジャンプバッファ（ジャンプボタンを早押ししても有効） | ||
| if (this.jumpBufferTime > 0) { | ||
| this.jumpBufferTime--; | ||
| if (this.grounded) { | ||
| this.jump(); | ||
| } | ||
| } | ||
|
|
||
| // 着地処理 | ||
| if (this.grounded && this.jumping) { | ||
| this.jumping = false; | ||
| this.canJump = true; | ||
| } | ||
|
|
||
| // 地面から離れた時のコヨーテタイム設定 | ||
| if (!this.grounded && !this.jumping && this.coyoteTime === 0) { | ||
| this.coyoteTime = 5; | ||
| } | ||
| } |
There was a problem hiding this comment.
タイマーの二重デクリメントバグ。
coyoteTime と jumpBufferTime が updateMovement() と updateTimers() の両方でデクリメントされています。update() が両方を呼び出すため、タイマーが意図した2倍の速度で減少します。
updateTimers() でのデクリメントを削除するか、updateMovement() 内のデクリメントを削除してください：
// タイマー更新
updateTimers() {
- if (this.jumpBufferTime > 0) {
- this.jumpBufferTime--;
- }
- if (this.coyoteTime > 0) {
- this.coyoteTime--;
- }
+ // タイマーのデクリメントは updateMovement() で行われるため、
+ // ここでは追加のタイマー処理のみ
}または updateMovement() からデクリメント処理を削除して updateTimers() に統合してください。
Also applies to: 187-195
1 participant
Summary
Testing
Codex Task
Summary by CodeRabbit
リリースノート
✏️ Tip: You can customize this high-level summary in your review settings.