[storybored379]

A plug-in effect gets the layer time in render (docs/plugins.md: "Time is layer time in seconds"), but the layer cache only folds the time into its key for effects listed in TIME_DEPENDENT or named ec.sim.*. Plug-in ids can't be either, so a plug-in that animates from time alone, with static parameters, was rendered once and that frame was reused at every other time.

is_time_dependent now also returns true for a registered plug-in id (new plugin::is_plugin, which reads the plug-in registry and tolerates a poisoned lock). Nothing tells the renderer whether a plug-in reads time, so this takes the safe default the issue suggests; an opt-out manifest flag could come later if caching static plug-ins matters. docs/plugins.md says so next to the time contract, and ROADMAP.md has a progress-log line.

Validation:

• New crates/host/tests/plugins.rs::time_dependent_plugins_render_each_frame registers a Rust plug-in that fills the frame with a grey equal to time and renders at 0 s, 0.5 s and 0 s again. It fails on main (9db7f3b) with the frame at 0.5 s reusing the 0 s frame, and passes with the fix; the other three plug-in tests (including the example WebAssembly plug-in) still pass.
• fmt, strict workspace Clippy (--all-targets --release -D warnings), layers, assets and WASM checks pass.
• Full cargo test --workspace --release (LTO off, 16 codegen units): 3,599 passed / 80 failed / 31 ignored. 79 failures are GPU tests (no usable GPU on this Linux box: "the GPU renders the frame") plus two FFmpeg oracle tests (AC-3 downmix gain, Radiance HDR fixture); 77 of them are the same set an earlier run on (older) main hit on this machine, and the other two are GPU tests too. The last one, tests_auto::auto_backend_moves_light_comps_to_the_cpu, is timing-based and passes when rerun alone on the loaded box. None of them registers a plug-in.

Fixes #1977 (the first problem it describes; the fuel/instance-pool note is separate).

Found and fixed with Claude Code (AI-assisted).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Plug-in effects now re-render for each frame when their output depends on time, preventing them from freezing on the first rendered frame.
  • Time-dependent plug-ins now correctly respond to changing layer time, including when no animated parameters are present.

• Documentation

  • Updated the plug-in contract to clarify that rendering may depend on layer time and occurs for every frame.

• Tests

  • Added coverage confirming time-dependent plug-ins produce the expected output across multiple render times.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: storytold/effectcraft/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: aa80891d-d201-44a0-88da-e74d2f589e56

📥 Commits

Reviewing files that changed from the base of the PR and between 02b7678 and 2908d6c.

📒 Files selected for processing: 1

• ROADMAP.md

🚧 Files skipped from review as they are similar to previous changes: 1

• ROADMAP.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Registered plug-in effects are now classified as time-dependent. A regression test checks their output at different render times. The plug-in timing documentation and revision history describe per-frame rendering.

Changes: Plug-in time-dependent rendering

Layer / File(s)	Summary
Classify registered plug-in effects as time-dependent crates/effects/src/plugin.rs, crates/effects/src/lib.rs	The plug-in registry now provides an exact-ID lookup. is_time_dependent uses it to classify registered plug-in effects as time-dependent.
Verify and document time-dependent rendering crates/host/tests/plugins.rs, docs/plugins.md, ROADMAP.md	A clock plug-in regression test checks output at different render times. The plug-in timing contract and revision history describe per-frame rendering.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: vangel76

Merge Risk: ⚪ Minimal · up to 2908d

No merge-blocking issue is identified in the selected change; it is ready for normal checks.

Security architecture risk: ⚪ Minimal · up to 02b76

The change corrects cached plug-in output across different render times without adding plug-in privileges or new loading paths. No material security risk was identified in the reviewed change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed execution frequency affects layers using registered plug-ins: different render times can invoke those plug-ins instead of reusing one frame. The inspected change does not grant additional capabilities or introduce another way to load a plug-in.

Trust Boundaries and Controls

• observed — Existing registration validates the manifest, rejects duplicate IDs, serializes changes with a write lock and initializes the plug-in slot before publishing its specification. The new helper only observes the published specifications and does not bypass those checks.

Resilience and Maintainability Implications

• observed — The existing render wrapper snapshots pixels and restores them when a plug-in returns an error. The PR leaves that failure-containment mechanism unchanged; it does not establish additional recovery guarantees for arbitrary native plug-in panics or process interruption.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: plug-in effects now re-render for each frame time.
Linked Issues check		Issue [#1977] requires plug-in effects that read time to render at different layer times with static parameters. is_time_dependent now recognizes every registered plug-in through `plugin::is_plugi…
Out of Scope Changes check		The changes stay within [#1977]. The implementation updates plug-in time-dependence detection, the test verifies cache behavior, and the documentation and roadmap entries describe the fix. No unrelate…

Full details: Linked Issues check

Explanation

Issue [#1977] requires plug-in effects that read time to render at different layer times with static parameters. is_time_dependent now recognizes every registered plug-in through plugin::is_plugin, so the existing layer-cache path includes layer time for Rust and WebAssembly plug-ins. The new host test verifies rendering at 0, 0.5, and 0 seconds. The manifest opt-out is optional guidance, not a required coding objective. The separate fuel and instance-pool note is not part of this objective.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#1977]. The implementation updates plug-in time-dependence detection, the test verifies cache behavior, and the documentation and roadmap entries describe the fix. No unrelated fuel, instance-pool, or other plug-in behavior changes are included.

Full details: Docstring Coverage

Explanation

Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[bflatastic]

Hi @storybored379, thanks for this PR! main has changed a lot since it was opened, and it now conflicts with main, so it needs a rebase before we can review it for merging.

Most of the conflict comes from the shared module layout (#2491), which moved or split files this PR touches:

• crates/host/tests/plugins.rs → crates/host/tests/it/plugins.rs

When you rebase, please also make sure it fits the other recent changes:

• Layout (refactor: shared module layout (craft-contrib#20) #2491): put new code in the split files (the old paths only re-export). Each crate's integration tests are one binary, tests/it/main.rs with modules in tests/it/.
• Strict clippy (Enforce Rust coding standards via Clippy #2496): cognitive_complexity (max 30), too_many_arguments and wildcard_imports are denied. Use craft_ui::ComboBox, not egui::ComboBox. Don't silence these with #[allow].
• egui 0.37 (Migrate EffectCraft to egui 0.37 #2367) and panels on craft_ui::docking (Panels: move onto craft-ui's shared docking #2432). Date and time conversion goes through chrono.

CI (cargo xtask ci) now runs on PRs. Once it's green on the rebased branch, we'll take a look. Thank you!