Address bghgary PR review feedback.

- AppContext: take PlaygroundOptions by value (defaulted to {}); drop the
  nullable-pointer dance and the six nullptr-guard branches in the ctor.
- Win32: rename g_options -> options to match the file's existing global
  naming convention; rename GetCommandLineArgumentsW ->
  GetCommandLineArguments since the W suffix collides with the Win32
  wide-char API convention (the function returns UTF-8 narrow strings).
- CommandLine: replace 13 repeated match/apply blocks with a static
  kFlags table whose rows describe long name, short name, kind, value
  label, help text, and an apply functor. PrintUsage walks the same
  table so flag definitions are no longer a second source of truth and
  adding a flag is a single-row edit.
- --list: emit canonical TSV (index, title, referenceImage,
  exclusionReason) so the listing is machine-parseable. The
  exclusionReason column reflects config state (ignores
  --include-excluded). Update help text and instruction docs to match.
- validation_native.js: drop the leading underscore on _opts and the
  five run-counters (_ranCount etc.); the prefix only carries meaning on
  _playgroundOptions, which is host-injected from C++. Remove the
  duplicate \if (breakOnFail) debugger;\ in the runner -- failTest()
  already breaks before invoking the done callback.
- instructions: replace the hard-coded test-index=286 example with a
  --test "<title>" form so config.json reorderings don't silently break
  the doc; drop hard-coded rdc / renderdoc-py version numbers.
- Revert renderdoc-gpu-debug.instructions.md to master: the diff was
  pure em-dash to '--' substitution and unrelated to debuggability.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: bkaradzic

.github/instructions/babylon-native-debugging.instructions.md

@@ -165,9 +165,11 @@ Substring match, case-insensitive, against `test.title`.
 
 ### Run one test by index (when titles collide or you only have a survey row)
 ```powershell
-.\Playground.exe --headless --once --test-index=286 `
+.\Playground.exe --headless --once --test-index=N `
     app:///Scripts/validation_native.js
 ```
+Use `--list` to find the index for a title. Indices are not stable across
+config.json edits; always re-resolve when the test list changes.
 
 ### Run a quarantined test without editing config.json
 ```powershell
@@ -183,8 +185,8 @@ debugging.
 ```powershell
 .\Playground.exe --list
 ```
-Prints TSV with index, title, exclusion reason. Pipe to `findstr` /
-`Select-String` for filtering.
+Prints TSV with index, title, referenceImage, exclusionReason. Pipe to
+`findstr` / `Select-String` for filtering.
 
 ### Pixel-diff investigation
 1. Run the failing test with `--headless --once --include-excluded`.
@@ -225,14 +227,14 @@ returns `true`, so the legacy throw-on-missing behavior is preserved.
 # Playground process before main, so bgfx::findModule adopts it. Version
 # always matches what `rdc open` accepts -- no PATH-ordering bugs.
 & "<renderdoc-py-dir>\renderdoccmd.exe" capture -w .\Playground.exe `
-    --headless --once --test-index=286 --include-excluded --capture=5 `
+    --headless --once --test "<test title or substring>" --include-excluded --capture=5 `
     app:///Scripts/validation_native.js
 # .rdc lives at <cwd>\temp\bgfx_frame5.rdc
 ```
 Equivalent with the rdc-cli wrapper (inject-only, no auto-capture handshake):
 ```powershell
 & rdc capture --trigger --wait-for-exit -- .\Playground.exe `
-    --headless --once --test-index=286 --include-excluded --capture=5 `
+    --headless --once --test "<test title or substring>" --include-excluded --capture=5 `
     app:///Scripts/validation_native.js
 ```
 Verify `renderdoc.dll` is loaded:

.github/instructions/playground/playground-renderdoc-capture.instructions.md

@@ -122,7 +122,7 @@ Just delete `<cwd>/temp/` if you want to free disk.
 ## What the rdc CLI **cannot** do (drop to renderdoc-py)
 
 - Show the IA input element layout (no `pipeline EID ia` section in
-  rdc 0.5.x with renderdoc-py 1.41).
+  current rdc).
 - Identify which buffer slot/offset feeds which shader semantic.
 - Show *all* components of VS output (only first 4 are dumped to JSON).
 - Debug shader execution (`rdc debug vertex/pixel` may crash the daemon
@@ -162,14 +162,14 @@ data = ctrl.GetBufferData(post.vertexResourceId,
 # GetAction(eid) on this version.
 ```
 
-## Worked example -- test 286 "Instances with color buffer"
+## Worked example -- "Instances with color buffer"
 
 Symptom: pixel `(13,13,15)` instead of expected `(255,0,0)`. Both cubes
 render black; reference shows red+green from per-instance color buffer.
 
 ```powershell
 & $rdcmd capture -w .\Playground.exe `
-    --once --test-index=286 --include-excluded --capture=5 `
+    --once --test "Instances with color buffer" --include-excluded --capture=5 `
     app:///Scripts/validation_native.js
 # -> "First pixel off at 286108: Value: (13, 13, 15) - Expected: (255, 0, 0)"
 # -> "Pixel difference: 53296 pixels."

.github/instructions/renderdoc/renderdoc-gpu-debug.instructions.md

@@ -1,6 +1,6 @@
 # RenderDoc GPU Debugging Instructions
 
-> Based on [rudybear/renderdoc-skill](https://github.com/rudybear/renderdoc-skill) -- adapted for GitHub Copilot.
+> Based on [rudybear/renderdoc-skill](https://github.com/rudybear/renderdoc-skill) — adapted for GitHub Copilot.
 
 ## When to Use
 
@@ -303,12 +303,12 @@ rdc diff capture_a.rdc capture_b.rdc --framebuffer --diff-output ./captures/anal
 
 For detailed step-by-step debugging workflows with expected output shapes, see `debugging-recipes.instructions.md`. Available recipes:
 
-- **Object is Invisible** -- culling, depth, blend, vertex transform checks
-- **Colors Are Wrong** -- texture bindings, constants, blend state, pixel shader trace
-- **Shadows Are Broken** -- shadow map resolution, depth bias, light matrices, PCF
-- **Performance Is Bad** -- draw counts, resource sizes, overdraw, state changes
-- **What Changed Between Two Frames** -- capture diff workflow
-- **Debug This Pixel** -- pixel history, shader trace, variable inspection
+- **Object is Invisible** — culling, depth, blend, vertex transform checks
+- **Colors Are Wrong** — texture bindings, constants, blend state, pixel shader trace
+- **Shadows Are Broken** — shadow map resolution, depth bias, light matrices, PCF
+- **Performance Is Bad** — draw counts, resource sizes, overdraw, state changes
+- **What Changed Between Two Frames** — capture diff workflow
+- **Debug This Pixel** — pixel history, shader trace, variable inspection
 
 ## 11. Output Size Management
 

Apps/Playground/Scripts/validation_native.js

@@ -1,21 +1,21 @@
 (function () {
     let currentScene;
     let config;
-    const _opts = (typeof _playgroundOptions === "object" && _playgroundOptions) ? _playgroundOptions : {};
-    const justOnce = !!_opts.runOnce;
-    const saveResult = (typeof _opts.saveResults === "boolean") ? _opts.saveResults : true;
+    const opts = (typeof _playgroundOptions === "object" && _playgroundOptions) ? _playgroundOptions : {};
+    const justOnce = !!opts.runOnce;
+    const saveResult = (typeof opts.saveResults === "boolean") ? opts.saveResults : true;
     const testWidth = 600;
     const testHeight = 400;
-    const generateReferences = !!_opts.generateReferences;
-    const breakOnFail = !!_opts.breakOnFail;
-    const listTests = !!_opts.listTests;
-    const includeExcluded = !!_opts.includeExcluded;
-    const testFilters = Array.isArray(_opts.testFilters) ? _opts.testFilters.map(s => String(s).toLowerCase()) : [];
-    const testIndices = Array.isArray(_opts.testIndices) ? _opts.testIndices.map(n => +n) : [];
+    const generateReferences = !!opts.generateReferences;
+    const breakOnFail = !!opts.breakOnFail;
+    const listTests = !!opts.listTests;
+    const includeExcluded = !!opts.includeExcluded;
+    const testFilters = Array.isArray(opts.testFilters) ? opts.testFilters.map(s => String(s).toLowerCase()) : [];
+    const testIndices = Array.isArray(opts.testIndices) ? opts.testIndices.map(n => +n) : [];
     // CLI --capture=N: 1-based frame index at which to call
     // TestUtils.captureNextFrame() for every executed test. The runner
     // extends each test's render budget so the .rdc finalizes.
-    const cliCaptureFrame = (typeof _opts.captureFrame === "number" && _opts.captureFrame > 0) ? (_opts.captureFrame | 0) : 0;
+    const cliCaptureFrame = (typeof opts.captureFrame === "number" && opts.captureFrame > 0) ? (opts.captureFrame | 0) : 0;
     // Frames after the trigger to let RenderDoc finalize the .rdc.
     const POST_CAPTURE_FRAMES = 5;
 
@@ -46,16 +46,13 @@
     }
 
     // Per-run counters surfaced as a final summary line on exit.
-    let _ranCount = 0;
-    let _passedCount = 0;
-    let _failedCount = 0;
-    let _skippedCount = 0;
-    let _missingRefCount = 0;
+    let ranCount = 0;
+    let passedCount = 0;
+    let failedCount = 0;
+    let skippedCount = 0;
+    let missingRefCount = 0;
 
-    function getSkipReason(t) {
-        if (includeExcluded) {
-            return null;
-        }
+    function getExclusionReason(t) {
         if (t.onlyVisual) {
             return "onlyVisual";
         }
@@ -68,12 +65,19 @@
         return null;
     }
 
+    function getSkipReason(t) {
+        if (includeExcluded) {
+            return null;
+        }
+        return getExclusionReason(t);
+    }
+
     function logRunSummary() {
-        console.log("Run complete. ran=" + _ranCount +
-                    " passed=" + _passedCount +
-                    " failed=" + _failedCount +
-                    " missingRef=" + _missingRefCount +
-                    " skipped=" + _skippedCount);
+        console.log("Run complete. ran=" + ranCount +
+                    " passed=" + passedCount +
+                    " failed=" + failedCount +
+                    " missingRef=" + missingRefCount +
+                    " skipped=" + skippedCount);
     }
 
     const engine = new BABYLON.NativeEngine();
@@ -448,7 +452,7 @@
                 if (!test.referenceImage) {
                     console.error("MISSING_REFERENCE_IMAGE: Test '" + (test.title || "(unnamed)") +
                                   "' has no 'referenceImage' field in config.json - cannot run pixel comparison.");
-                    _missingRefCount++;
+                    missingRefCount++;
                     failTest(done);
                     return;
                 }
@@ -457,7 +461,7 @@
                     console.error("MISSING_REFERENCE_IMAGE: Test '" + (test.title || "(unnamed)") +
                                   "' reference image not found at app:///ReferenceImages/" +
                                   test.referenceImage + " - cannot run pixel comparison.");
-                    _missingRefCount++;
+                    missingRefCount++;
                     failTest(done);
                     return;
                 }
@@ -515,9 +519,13 @@
             config = JSON.parse(xhr.responseText);
 
             if (listTests) {
+                // Canonical TSV: index<TAB>title<TAB>referenceImage<TAB>exclusionReason.
+                // exclusionReason reflects config state (ignores --include-excluded)
+                // so the listing is the same regardless of run flags.
                 for (let i = 0; i < config.tests.length; ++i) {
                     const t = config.tests[i];
-                    console.log("[" + i + "] " + (t.title || "") + " -> " + (t.referenceImage || ""));
+                    const reason = getExclusionReason(t) || "";
+                    console.log(i + "\t" + (t.title || "") + "\t" + (t.referenceImage || "") + "\t" + reason);
                 }
                 engine.dispose();
                 TestUtils.exit(0);
@@ -544,7 +552,7 @@
                     const reason = getSkipReason(t);
                     if (reason !== null) {
                         console.log("Skipping '" + (t.title || "(unnamed)") + "' -- " + reason);
-                        _skippedCount++;
+                        skippedCount++;
                         i++;
                         continue;
                     }
@@ -553,25 +561,21 @@
                 if (i >= config.tests.length) {
                     logRunSummary();
                     engine.dispose();
-                    TestUtils.exit(_failedCount > 0 ? -1 : 0);
+                    TestUtils.exit(failedCount > 0 ? -1 : 0);
                     return;
                 }
                 const currentTitle = config.tests[i].title || "(unnamed)";
                 runTest(i, function (status) {
-                    _ranCount++;
+                    ranCount++;
                     if (!status) {
-                        _failedCount++;
-                        // Inner failures fire the debugger via failTest();
-                        // this catches the comparison-failed path.
-                        if (breakOnFail) {
-                            // eslint-disable-next-line no-debugger
-                            debugger;
-                        }
+                        failedCount++;
+                        // failTest() already triggered the debugger before
+                        // reaching this callback; no second `debugger` here.
                         logRunSummary();
                         TestUtils.exit(-1);
                         return;
                     }
-                    _passedCount++;
+                    passedCount++;
                     i++;
                     if (justOnce || i >= config.tests.length) {
                         logRunSummary();

Apps/Playground/Shared/AppContext.cpp

@@ -52,15 +52,15 @@ AppContext::AppContext(
     size_t height,
     DebugLogCallback debugLog,
     AdditionalInitCallback additionalInit,
-    const PlaygroundOptions* playgroundOptions)
+    PlaygroundOptions playgroundOptions)
 {
-    Babylon::DebugTrace::EnableDebugTrace(playgroundOptions == nullptr || !playgroundOptions->DebugTrace.has_value() ? true : *playgroundOptions->DebugTrace);
+    Babylon::DebugTrace::EnableDebugTrace(playgroundOptions.DebugTrace.value_or(true));
     Babylon::DebugTrace::SetTraceOutput(debugLog);
 
     Babylon::PerfTrace::Level perfLevel{Babylon::PerfTrace::Level::Mark};
-    if (playgroundOptions != nullptr && playgroundOptions->PerfTrace.has_value())
+    if (playgroundOptions.PerfTrace.has_value())
     {
-        const auto& v = *playgroundOptions->PerfTrace;
+        const auto& v = *playgroundOptions.PerfTrace;
         if (v == "None" || v == "none")
         {
             perfLevel = Babylon::PerfTrace::Level::None;
@@ -118,38 +118,37 @@ AppContext::AppContext(
 
     m_runtime.emplace(options);
 
-    m_runtime->Dispatch([this, window, debugLog, additionalInit = std::move(additionalInit), playgroundOptions](Napi::Env env) {
+    m_runtime->Dispatch([this, window, debugLog, additionalInit = std::move(additionalInit), playgroundOptions = std::move(playgroundOptions)](Napi::Env env) {
         m_device->AddToJavaScript(env);
 
-        if (playgroundOptions != nullptr)
         {
             auto js = Napi::Object::New(env);
-            js.Set("listTests",          Napi::Boolean::New(env, playgroundOptions->ListTests));
-            js.Set("headless",           Napi::Boolean::New(env, playgroundOptions->Headless));
-            js.Set("breakOnFail",        Napi::Boolean::New(env, playgroundOptions->BreakOnFail));
-            js.Set("generateReferences", Napi::Boolean::New(env, playgroundOptions->GenerateReferences));
-            js.Set("runOnce",            Napi::Boolean::New(env, playgroundOptions->RunOnce));
-            js.Set("includeExcluded",    Napi::Boolean::New(env, playgroundOptions->IncludeExcluded));
-            if (playgroundOptions->SaveResults.has_value())
+            js.Set("listTests",          Napi::Boolean::New(env, playgroundOptions.ListTests));
+            js.Set("headless",           Napi::Boolean::New(env, playgroundOptions.Headless));
+            js.Set("breakOnFail",        Napi::Boolean::New(env, playgroundOptions.BreakOnFail));
+            js.Set("generateReferences", Napi::Boolean::New(env, playgroundOptions.GenerateReferences));
+            js.Set("runOnce",            Napi::Boolean::New(env, playgroundOptions.RunOnce));
+            js.Set("includeExcluded",    Napi::Boolean::New(env, playgroundOptions.IncludeExcluded));
+            if (playgroundOptions.SaveResults.has_value())
             {
-                js.Set("saveResults", Napi::Boolean::New(env, *playgroundOptions->SaveResults));
+                js.Set("saveResults", Napi::Boolean::New(env, *playgroundOptions.SaveResults));
             }
-            if (playgroundOptions->CaptureFrame.has_value())
+            if (playgroundOptions.CaptureFrame.has_value())
             {
-                js.Set("captureFrame", Napi::Number::New(env, *playgroundOptions->CaptureFrame));
+                js.Set("captureFrame", Napi::Number::New(env, *playgroundOptions.CaptureFrame));
             }
 
-            auto filters = Napi::Array::New(env, playgroundOptions->TestFilters.size());
-            for (uint32_t i = 0; i < playgroundOptions->TestFilters.size(); ++i)
+            auto filters = Napi::Array::New(env, playgroundOptions.TestFilters.size());
+            for (uint32_t i = 0; i < playgroundOptions.TestFilters.size(); ++i)
             {
-                filters[i] = Napi::String::New(env, playgroundOptions->TestFilters[i]);
+                filters[i] = Napi::String::New(env, playgroundOptions.TestFilters[i]);
             }
             js.Set("testFilters", filters);
 
-            auto indices = Napi::Array::New(env, playgroundOptions->TestIndices.size());
-            for (uint32_t i = 0; i < playgroundOptions->TestIndices.size(); ++i)
+            auto indices = Napi::Array::New(env, playgroundOptions.TestIndices.size());
+            for (uint32_t i = 0; i < playgroundOptions.TestIndices.size(); ++i)
             {
-                indices[i] = Napi::Number::New(env, playgroundOptions->TestIndices[i]);
+                indices[i] = Napi::Number::New(env, playgroundOptions.TestIndices[i]);
             }
             js.Set("testIndices", indices);
 

Apps/Playground/Shared/AppContext.h

@@ -26,7 +26,7 @@ class AppContext
         size_t height,
         DebugLogCallback debugLog,
         AdditionalInitCallback additionalInit = {},
-        const PlaygroundOptions* playgroundOptions = nullptr);
+        PlaygroundOptions playgroundOptions = {});
 
     ~AppContext();