Add landscape canopy and patch primitives#79
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request introduces two new landscape components, TreeCanopy and OrganicPatch, to the component plan specification. It includes comprehensive documentation, schema definitions, validation rules, expansion logic, and tests for both components, along with a new example study. Feedback on the implementation highlights two issues in packages/core/src/componentPlan.ts: first, outputPart unconditionally returns "fill_0" for OrganicPatch, which can cause errors if the first row is completely trimmed away; second, when includeBorder is enabled, rows with zero trim will not generate border blocks, resulting in gaps in the border.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| case "OrganicPatch": | ||
| return "fill_0"; |
There was a problem hiding this comment.
If the first row of an OrganicPatch (at z = 0) is completely trimmed away (which can happen when roughness is high relative to the width), fill_0 will be skipped and won't be emitted in the placements. However, outputPart still returns "fill_0" unconditionally. This will cause compilation or graph errors for any other components referencing this OrganicPatch because they will try to connect to a non-existent node ID.\n\nTo prevent this, outputPart should dynamically return the first actually emitted part of the patch, falling back to "fill_0" only if the patch is empty.
case "OrganicPatch": {\n const placements = organicPatchPlacements(component);\n return placements.length > 0 ? placements[0].part : "fill_0";\n }| for (let z = 0; z < size.length; z += 1) { | ||
| const trim = organicRowTrim(z, size.length, roughness); |
There was a problem hiding this comment.
When includeBorder is enabled, the border blocks are placed in the trimmed space (i.e., at anchor.x + trim - 1 and anchor.x + trim + width). However, if trim is 0 (which occurs on rows where no trimming is applied), no border blocks are generated for that row. This results in visible gaps in the border of the patch (e.g., a pond with missing stone borders on some rows).\n\nTo ensure a continuous and complete border, we should guarantee a minimum trim of 1 block on all rows when includeBorder is true.
for (let z = 0; z < size.length; z += 1) {\n const rawTrim = organicRowTrim(z, size.length, roughness);\n const trim = includeBorder && rawTrim === 0 ? 1 : rawTrim;
Summary
Part of #73
Tests