Repository navigation
Make the docs say what the code does about modifiers and lifecycle - #163
Conversation
Two places described behavior the code no longer has, the lifecycle hook names and where live modifiers run, and the fixture slots gain the rule for how they widen a layer. **Light domain** - ModifierBase.h, the class comment that feeds the modifier catalog: static modifiers fold in child order into the mapping; a dynamic modifier runs a pass every frame after all effects, on the logical buffer the static mapping then carries to the lights, whatever its place. The appendix's fold covers each enabled static modifier. - FixtureChannels.h: a slot past the layer's width widens the layer, and `setChannelsPerLight` rejects zero. The appendix is cut to one line per paragraph and each section gets an `@xref` from the member it explains, so the file leaves the docgen warning list. **Docs/CI** - moonmodule.md: `applyState()` calls `prepare()` and `release()`, the names since fa3066a. - moonlight.md: static modifiers chain in child order; the live pass runs after the effects, before the static mapping reaches the lights. - help.json regenerated for the ModifierBase comment. **Tests** - build_docs --strict: PASS. build_desktop: PASS, no warnings. check_code: PASS, counts unchanged. test_desktop: PASS, 2,844 test cases, 179,476 assertions. - check_specs: PASS. check_docgen: PASS, 0 errors, warnings 2050 → 2043. - check_esp32_built --firmware esp32s3-n16r8: PASS, rebuilt with these sources, no warnings. - collect_kpi: SKIP, comments and docs only. - check_prose: NegatedHeading 6→8 and SelfReference 17→18 against the committed counts with no change of ours, the same rise in the other branch, which changes no prose (Vale 3.25.1); left for the maintainer. **Reviews** Iterative review of all efforts using repo guides. - Fable review, round one: "after the static fold" read backwards for a catalog reader, done; the triggered checks listed, done; "Static modifiers are reorderable" narrowed a true sentence, done; a sentence in past tense, done. - Fable review, round two: a rebuild frame shows black, since `prepare()` clears the buffer, not the static result, so the sentence is cut; the appendix's fold named every modifier, done; "four places" corrected to two corrections and an addition, done; one word, live, for one concept in moonlight.md, done; the S3 build rebuilt rather than skipped, done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
There was a problem hiding this comment.
🟡 Changes recommended
The mirror/rotate example incorrectly claims that reordering the modifiers changes output, even though separate static and live processing stages leave the result unchanged.
1 open issue
What changed in this PR
This documentation-only PR aligns MoonLight’s lifecycle and modifier descriptions with the existing implementation. It also documents fixture-channel width behavior; runtime code is unchanged.
Changes:
- Updates lifecycle hook names to
prepare()andrelease(). - Distinguishes static modifier folding from per-frame live passes and regenerates help text.
- Documents layer widening and zero-width rejection, while condensing fixture-channel comments.
| File | Description |
|---|---|
src/light/util/FixtureChannels.h |
Clarifies channel-slot behavior and adds appendix cross-references. |
src/light/modifiers/ModifierBase.h |
Corrects static and live modifier descriptions. |
docs/help/help.json |
Regenerates modifier help from the updated comments. |
docs/explanation/architecture/moonmodule.md |
Uses current resource-lifecycle hook names. |
docs/explanation/architecture/moonlight.md |
Documents separate static and live modifier stages. |
Review effort: Balanced
Order composes the static modifiers only, so the example that shows it names two of them. **Docs/CI** - moonlight.md: order is meaningful among the static modifiers, shown by multiply and checkerboard; the mirror and rotate pair goes, since Rotate is live and runs after the static chain wherever it sits. **Reviews** - Copilot: the mirror/rotate example contradicted the static/live rule, since swapping a live Rotate with a static Mirror changes nothing; done, the example names two static modifiers.
|
Merged, thanks James! Every claim checked out against the code: live modifiers skip the static fold and run after the effects, and a slot past the width widens the layer in |
Two places where the docs describe behavior the code no longer has, and one rule they never stated, found while mapping the repository for a maintainer's index:
moonmodule.mdstill named the lifecycle hooksonBuildState()andteardown(), renamed toprepare()andrelease()in fa3066a.Layer.haround lines 365 and 441) and run every frame after all effects, on the logical buffer the static mapping then carries to the lights. The catalog intro, its appendix andmoonlight.mdnow say so.setChannelsPerLightrejects zero.Comments and docs only, plus the regenerated help.json. FixtureChannels.h is cut to one line per paragraph with an
@xrefinto each appendix section, since the change touched it.One thing found on the way and left out of this PR: a frame on which a modifier asks for a rebuild re-prepares the layer, and
Layer::prepare()clears the buffer, so that frame composites black. If an animated modifier rebuilds on its own clock, that may show as a flicker. Not reproduced; worth a look on its own.What an agent wrote: Claude found the mismatches and drafted the edits; two independent agent reviews checked them against the code; I verified each and reviewed every line.
Tested: desktop: build_docs --strict, build_desktop and test_desktop, check_code, check_docgen, check_specs; the esp32s3-n16r8 firmware builds. check_prose reports a rise in two rules against the committed counts that this branch does not cause (details in the commit). Not flashed: comments and docs only.