Carnival - #212
Conversation
📁 Previous CI results (run #20414630662)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ❌The PR changes files that should not be changed:
Please ensure that only allowed files are modified. Any changes in the Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2025-12-21 19:23:56 UTC. |
851a69b to
723cf82
Compare
📁 Previous CI results (run #20414689970)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ❌The PR changes files that should not be changed:
Please ensure that only allowed files are modified. Any changes in the Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2025-12-21 19:25:33 UTC. |
📁 Previous CI results (run #20415348235)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ✅There are no forbidden file changes. Nice 👍 Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2025-12-21 19:30:24 UTC. |
📁 Previous CI results (run #20931717063)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ✅There are no forbidden file changes. Nice 👍 Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2025-12-21 20:30:31 UTC. |
|
Latest commit is green (base=dev, changed-files ok, framework tests passed). One workflow run is awaiting maintainer approval due to fork security (Internal PR Tests). Please approve if it is required. |
Code Review: CarnivalSummaryI've spent some time playing through your Carnival implementation and comparing it to the original Atari version, and I have to say the JAX implementation itself is really solid. The code is clean, well-structured, and properly uses JAX primitives throughout. However, I found some significant gameplay differences that really change how the game feels and plays compared to the original. Of course i understand that this is a purely functional submission without concentrating on visuals, but i will mention them anyway, maybe this helps to organize later. The biggest issue is that there's no way to replenish ammunition, which makes the game basically unwinnable. ALE Similarity: 1.5/4Strengths
Issues Found1. Missing Ammunition Replenishment System
2. Incorrect Falling Object Behavior
3. Simplified Visuals and Missing Features
Recommendations
Implementation Quality: 3.5/4Strengths
Issues FoundHonestly, from a pure JAX implementation perspective, I didn't find any issues. The code quality is great. The problems are all in the game logic itself (the missing mechanics I mentioned above), not in how you've written the JAX code. Overall AssessmentTotal Score: (ALE: 1.5/4) + (Implementation: 3.5/4) = 5/8 Key Strengths
Priority Improvements
Final ThoughtsI really like the JAX foundation you've built. Adding more sprites from the original game would improve your implementation. So your challenge now is getting the gameplay to match the original ALE version. Fix the ammo system first since that's what's breaking the game right now. Great work on the implementation! |
Review - CarnivalFor this Review we played your game for about an hour, compared it side by side with the ALE version and read over your code implementation. Sínce this is an early stage of development, we know that you will change many things anyway. We considered you thought about your game implementation and how you want to do certain things a lot more than we do, but since this is a review we might still point out the obvious and hope we can give you reassurance about the things you wanted to change or build. Our game is also not ready yet, so we tried to write our review with this in mind, because we can probably relate how you would think right now. We hope this review helps a little bit, to finalize your game. :) ALE Similarity (2/4): What’s going well:
What’s different or missing:
Implementation Quality (3/4): Well implemented parts:
Recommendations:
|
|
Thank you both for the thorough reviews and for taking the time to play the game, compare it side by side with ALE, and read through the code. I really appreciate how specific and actionable your feedback is. I’m glad the JAX side of the implementation came across well. I intentionally focused first on getting a clean, fully JIT-compatible, pure-functional structure with modular helpers, because that is the foundation I need for fast iteration and future fidelity work. I completely agree with the main conclusion from both of you: there are still several gameplay mechanics that do not match the original Carnival, and those gaps change the feel of the game in a big way. I will update the mismatches and required mechanics as soon as possible. My priority list is: 1) Ammo replenishment (critical) 2) Falling duck / chicken behavior and penalties 3) Input mapping bug 4) Timing and speeds
However, you’re absolutely right that for ALE similarity the default timing is what really matters. If the bullet is too slow relative to the moving rows, the hit window shifts and the player has to over-anticipate shots. I’ll tune the default bullet/row/player/fall speeds to be closer to the original ALE dynamics. 5) Target row patterns and initial fill Visual polish, additional target variety, and sprite fidelity are also on my list, but I agree those should come after the core mechanics above, since these directly affect playability and similarity. Thanks again for the detailed feedback. I’ll push updates addressing these points as soon as possible. |
📁 Previous CI results (run #20932891324)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ❌The PR changes files that should not be changed:
Please ensure that only allowed files are modified. Any changes in the Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2026-01-12 19:14:54 UTC. |
📁 Previous CI results (run #20933161808)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ❌The PR changes files that should not be changed:
Please ensure that only allowed files are modified. Any changes in the Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2026-01-12 19:55:16 UTC. |
c70d727 to
c3c297e
Compare
📁 Previous CI results (run #22093777834)Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ✅There are no forbidden file changes. Nice 👍 Framework Tests ✅All framework tests passed. Good work! 🎉 This log was automatically created at 2026-01-12 20:05:20 UTC. |
Test ReportThis comment was generated automatically by a GitHub Action. It summarizes the test results for this pull request. The GitHub Action run can be found here: Base Branch ✅The PR's base branch is Changed Files ✅There are no forbidden file changes. Nice 👍 Framework Tests ❌Some framework tests failed. Please check the details below: carnival ❌This log was automatically created at 2026-02-17 10:20:29 UTC. |
Summary
Files
Notes
Testing