feat: implement math marauders prototype #4
Conversation
Summary of ChangesHello @cs-util, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the initial playable prototype for 'Math Marauders,' a wave-based runner game. It establishes a highly modular and extensible architecture, separating concerns into distinct systems for core game logic, user interface, data handling, and analytics. The changes include a fully functional HTML front-end for player interaction and a comprehensive suite of development tools to ensure code quality and maintainability from the outset. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request lays a strong foundation for the Math Marauders game by implementing the core modular systems for gameplay, UI, and persistence. The code is well-structured, leveraging modern JavaScript features and good practices like dependency injection. I've identified a critical issue in the main game loop where events don't fire in the correct sequence, a high-severity bug in the scoring logic, and a couple of medium-severity items related to test correctness and CSS practices. My detailed comments include code suggestions to address these points. Overall, this is an excellent start to the project.
| async processForwardPhase() { | ||
| this.uiManager.setPhase("forward"); | ||
| const skirmishByGate = new Map( | ||
| this.currentWave.skirmishes.map((skirmish) => [ | ||
| skirmish.gateId, | ||
| skirmish.enemySize, | ||
| ]), | ||
| ); | ||
| const result = await this.gateSystem.processGates( | ||
| this.currentWave.forward.map((gate) => ({ | ||
| ...gate, | ||
| total: this.currentWave.forward.length, | ||
| })), | ||
| this.currentWave.initialArmy, | ||
| "forward", | ||
| ); | ||
|
|
||
| result.history.forEach((entry) => { | ||
| const enemy = skirmishByGate.get(entry.gateId) ?? 0; | ||
| this.battleSystem.resolveSkirmish(enemy); | ||
| }); | ||
| return { ...result }; | ||
| } | ||
|
|
||
| async processRetreatPhase(initialArmy) { | ||
| this.uiManager.setPhase("retreat"); | ||
| this.flockSystem.setPlayerCount(initialArmy); | ||
| this.flockSystem.setEnemyCount( | ||
| Math.round(this.currentWave.optimal.retreat.final * 1.1), | ||
| ); | ||
| const result = await this.gateSystem.processGates( | ||
| this.currentWave.retreat.map((gate) => ({ | ||
| ...gate, | ||
| total: this.currentWave.retreat.length, | ||
| })), | ||
| initialArmy, | ||
| "retreat", | ||
| ); | ||
|
|
||
| result.history.forEach((entry) => { | ||
| this.battleSystem.triggerSurge(entry.gateId); | ||
| }); | ||
|
|
||
| return { ...result }; | ||
| } |
There was a problem hiding this comment.
There's a critical logic issue in how game phases are processed. Both processForwardPhase and processRetreatPhase first complete all gate interactions via gateSystem.processGates, and only then iterate through the history to trigger subsequent events (skirmishes or surges). According to the README.md, these events should happen immediately after each gate is cleared.
This implementation means that skirmishes in the forward phase don't use the player's army count as it changes from gate to gate, and surges in the retreat phase are not triggered in real-time.
To fix this, you should refactor these processing loops in GameController to handle one gate at a time, update the flockSystem, and then trigger the associated event immediately within the loop before proceeding to the next gate. This might involve moving the gate processing loop from GateSystem into GameController.
| export function calculateStars(actual, optimal) { | ||
| if (optimal <= 0) { | ||
| return 0; | ||
| } | ||
| const ratio = actual / optimal; | ||
| let stars = 0; | ||
| STAR_THRESHOLDS.forEach((threshold) => { | ||
| if (ratio >= threshold) { | ||
| stars += 1; | ||
| } | ||
| }); | ||
| return Math.min(5, stars + 1); | ||
| } |
There was a problem hiding this comment.
The star calculation logic doesn't align with the percentages specified in README.md. For example, a ratio of 0.4 (40%) should result in 1 star, but the current code returns 2 stars because of ratio >= threshold.
The README specifies ranges like 0-40% for 1 star and 41-60% for 2 stars. A clearer and more correct implementation would use a series of if statements to handle these ranges correctly.
export function calculateStars(actual, optimal) {
if (optimal <= 0) {
return 0;
}
const ratio = actual / optimal;
if (ratio > 0.9) return 5;
if (ratio > 0.75) return 4;
if (ratio > 0.6) return 3;
if (ratio > 0.4) return 2;
return 1;
}| <div | ||
| style=" | ||
| display: flex; | ||
| gap: 0.5rem; | ||
| justify-content: center; | ||
| flex-wrap: wrap; | ||
| " | ||
| > | ||
| <button id="popup-next">Next Wave</button> | ||
| <button id="popup-retry">Retry</button> | ||
| </div> |
There was a problem hiding this comment.
This div uses an inline style attribute. It's better to move these styles into a CSS class in the <style> block to improve maintainability and separate concerns. This also makes the styling more reusable.
You can add a class like popup__actions to the div and define the styles in the <style> block:
.popup__actions {
display: flex;
gap: 0.5rem;
justify-content: center;
flex-wrap: wrap;
} <div class="popup__actions">
<button id="popup-next">Next Wave</button>
<button id="popup-retry">Retry</button>
</div>| test("assigns correct thresholds", () => { | ||
| expect(calculateStars(40, 100)).toBe(2); | ||
| expect(calculateStars(60, 100)).toBe(3); | ||
| expect(calculateStars(75, 100)).toBe(4); | ||
| expect(calculateStars(90, 100)).toBe(5); | ||
| expect(calculateStars(100, 100)).toBe(5); | ||
| }); |
There was a problem hiding this comment.
These tests validate the current scoring logic, which has a bug regarding boundary values. Once you fix calculateStars, these tests will fail. They should be updated to reflect the correct scoring behavior as per the README.md, checking the boundaries of each star rating.
test("assigns correct thresholds", () => {
expect(calculateStars(40, 100)).toBe(1);
expect(calculateStars(41, 100)).toBe(2);
expect(calculateStars(60, 100)).toBe(2);
expect(calculateStars(61, 100)).toBe(3);
expect(calculateStars(75, 100)).toBe(3);
expect(calculateStars(76, 100)).toBe(4);
expect(calculateStars(90, 100)).toBe(4);
expect(calculateStars(91, 100)).toBe(5);
});![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| result.history.forEach((entry) => { | ||
| const enemy = skirmishByGate.get(entry.gateId) ?? 0; | ||
| this.battleSystem.resolveSkirmish(enemy); | ||
| }); |
There was a problem hiding this comment.
During the forward phase the loop over result.history only calls battleSystem.resolveSkirmish(enemy) and never applies the gate result (entry.after) to the flock before the skirmish or adjusts the returned finalCount. The retreat phase later seeds processRetreatPhase with forwardResult.finalCount, which is still the value produced by the gate math and ignores all skirmish casualties. This means enemy encounters can never reduce the survivor total or cause a wave failure—the player always proceeds with the unmodified gate result regardless of losses shown in the UI. Consider updating the flock and the forward result with the actual gate outcomes before each skirmish so casualties meaningfully affect success.
Useful? React with 👍 / 👎.
Summary
index.htmlTesting
https://chatgpt.com/codex/tasks/task_e_68e37728ec74832c89d0b7000dc8e5eb