editor_screenshot: address Copilot review on #456

[dsarno]

Follow-up to #456 addressing the three Copilot review findings.

Fixes

1. viewport_screenshot_precheck no longer false-rejects on root-type alone

The previous check if scene_root is Node3D: return {} rejected any scene whose root wasn't a Node3D, but it's perfectly valid (and common) for a scene to have a plain Node or Node2D root with Node3D descendants — the 3D viewport can still render those. New behavior: DFS the scene tree for any Node3D content, short-circuit on the first hit; reject only when the scene has no Node3D anywhere.

The hint copy also updated to say "no Node3D descendants" / "no Node3D content anywhere in the tree" instead of implying the root type itself is the problem.

2. Docstring error.data shape now matches reality

#456's review fix dropped hint from error.data (the hint text lives in error.message), but the tool docstring still listed hint as a data key. Updated to error.data = {editor_state: "viewport_not_3d", scene_root_type} with a note that the actionable hint is in error.message. Prevents schema-aware clients from looking for a nonexistent field.

3. Docstring "cinematic" no longer implies current is required

The old wording said NODE_NOT_FOUND if none is marked current. The actual handler (_find_current_camera_3d) prefers a Camera3D marked current but falls back to the first Camera3D found in DFS — NODE_NOT_FOUND only fires when the scene has zero Camera3Ds. Reworded:

Prefers a Camera3D marked current; falls back to the first Camera3D found in a depth-first walk. NODE_NOT_FOUND only when the scene contains no Camera3D at all.

Tests

5 new GDScript cases on top of the existing precheck suite:

• test_viewport_precheck_passes_for_plain_node_root_with_3d_descendant — the main false-reject scenario from finding Add Phase 2 scene + node write tools (Batches 5-6) #3
• test_viewport_precheck_passes_for_node2d_root_with_3d_descendant — Node2D wrapper around a 3D preview
• test_viewport_precheck_rejects_node2d_root_with_only_2d_descendants — confirms the original 2D-scene problem still rejects
• test_viewport_precheck_walks_deep_descendants — DFS reaches nested Node3D content, not just direct children
• The existing plain-Node test renamed to ..._with_no_3d_descendants and kept as the negative case

Pre-commit

• ruff check src/ tests/ — clean
• pytest -q — 1081 passed, 4 skipped (9 more than editor_screenshot: stop returning INTERNAL_ERROR on 2D/empty viewports #456 baseline from the new precheck DFS coverage in the existing tests + Python relay tests still intact)
• GDScript live smoke skipped — my plugin-managed dev server lost its session in the earlier multi-agent contention; the change is well-covered by the static-helper tests and CI will exercise GDScript across all three OSes on this PR.

Risk

Minimal — the precheck only loosens (fewer false rejects), never tightens. Worst case: a scene with a Node3D node that happens to be unrenderable (e.g. all visible=false) would now pass the precheck and hit the downstream empty-image guard, which now also returns EDITOR_NOT_READY with structured data per #456. So even that edge case fails gracefully.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!