Greenfield/hol v1 blueprint - #86
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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.
Reviewed commit: 33984fc55a
ℹ️ 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 "@codex address that feedback".
| - guess equals secret: Correct | ||
|
|
||
| ### Lock | ||
| - one Lock per player per match after account unlock |
There was a problem hiding this comment.
Players can enter online DUEL at level 3 without ever playing the AI mode, while Lock is only introduced after an unspecified number of DUEL turns. In that scenario, an established opponent has a match-deciding Lock that the new player lacks: a locked hit wins an otherwise tied round, so the blueprint's supposedly fair competitive outcome depends on account progression. Make Lock available to both players before online matchmaking, or gate/match players so both sides have identical rule access.
Useful? React with 👍 / 👎.
| ## 2. UX principles | ||
| - one dominant action per state | ||
| - important numbers and choices read instantly | ||
| - minimum interaction target equivalent to 44 px |
There was a problem hiding this comment.
On the specified 1080×1920 reference canvas, a literal 44-pixel target can be only about one-third of the customary 44-point/48-dp mobile minimum on high-density phones, making primary controls unnecessarily difficult to tap. Specify 44 points for iOS and at least 48 dp for Android, then translate those physical targets into the Unity reference-canvas dimensions rather than treating 44 canvas pixels as sufficient.
Useful? React with 👍 / 👎.
| - `HOL.Tests.EditMode` | ||
| - `HOL.Tests.PlayMode` | ||
|
|
||
| `HOL.Core` must have no UnityEngine dependency where practical. |
There was a problem hiding this comment.
The qualifier where practical lets an implementer place UnityEngine dependencies in HOL.Core, despite this document calling it a pure rules assembly and requiring enforced dependency direction. That would prevent the proposed core from remaining independently testable and violates the repository's explicit no-engine assembly boundary; make this an unconditional constraint and put Unity adapters in Infrastructure or Presentation.
AGENTS.md reference: AGENTS.md:L154-L158
Useful? React with 👍 / 👎.
| 7. `07_QA_CI_RELEASE_AND_DOD.md` | ||
| 8. `08_IMPLEMENTATION_ROADMAP.md` |
There was a problem hiding this comment.
The source-of-truth order names 07_QA_CI_RELEASE_AND_DOD.md and 08_IMPLEMENTATION_ROADMAP.md, but neither file exists anywhere in the repository; a repo-wide search finds only these two references. Consequently, consumers of this purported build pack have no defined QA/release completion criteria or implementation sequence, and cannot follow the declared source hierarchy. Add the documents or remove them from the authoritative list until they exist.
Useful? React with 👍 / 👎.
1 participant
No description provided.