fix: vessel-switch stale camera reattach + docking camera list

Plugin/Kerbcast/KerbcastCore.cs

@@ -268,12 +268,19 @@ private void OnPartDestroyed(Part part)
             if (affected != null) MarkFxDirtyForVessel(affected);
         }
 
-        // Decoupling, staging, fairing jettison etc. change a vessel's part set
-        // without firing onPartDestroyed for our tracked part. Rebuild the FX
-        // draw lists so they match the current parts.
+        // Decoupling, staging, docking, fairing jettison etc. change a
+        // vessel's part set without firing onVesselChange (no pilot switch)
+        // or onPartDestroyed for our tracked parts. Pick up any newly-joined
+        // parts (e.g. a docking partner's cameras) immediately rather than
+        // waiting for an incidental later vessel switch — additive only, see
+        // RebuildCameraList's disposeMissing doc comment for why staged-off
+        // debris must not be torn down here. Also rebuild the FX draw lists
+        // so they match the current parts.
         private void OnVesselWasModified(Vessel v)
         {
-            if (v != null) MarkFxDirtyForVessel(v);
+            if (v == null) return;
+            RebuildCameraList(v, disposeMissing: false);
+            MarkFxDirtyForVessel(v);
         }
 
         private void MarkFxDirtyForVessel(Vessel v)
@@ -306,10 +313,24 @@ private static uint SyntheticFlightId(uint baseId, int moduleIdx, string cameraN
             }
         }
 
-        private void RebuildCameraList(Vessel vessel)
+        // disposeMissing=true (onVesselChange — an actual pilot switch): scope
+        // the camera list to `vessel`, tearing down anything not on it. This
+        // is the documented "leaving a vessel drops its cameras" behavior.
+        //
+        // disposeMissing=false (onVesselWasModified — dock/stage without a
+        // pilot switch): additive only. A part leaving `vessel` via staging
+        // is still physically out there on debris and should only stop
+        // streaming when it's actually destroyed (physics range /
+        // onPartDestroyed), not because this vessel's membership changed —
+        // so nothing already tracked gets touched, and only cameras for
+        // newly-appeared parts (e.g. a docking partner's) get added.
+        private void RebuildCameraList(Vessel vessel, bool disposeMissing = true)
         {
-            foreach (var cam in _cameras) cam.Dispose();
-            _cameras.Clear();
+            if (disposeMissing)
+            {
+                foreach (var cam in _cameras) cam.Dispose();
+                _cameras.Clear();
+            }
 
             if (vessel == null) return;
 
@@ -365,6 +386,11 @@ private void RebuildCameraList(Vessel vessel)
                         // get a deterministic hash of (flightID, cameraName)
                         // so they're stable across loads.
                         uint flightId = SyntheticFlightId(part.flightID, moduleIdx, hullcam.cameraName);
+                        if (!disposeMissing && _cameras.Any(c => c.FlightId == flightId))
+                        {
+                            moduleIdx++;
+                            continue;
+                        }
                         var panCap = PartCapabilities.ForPart(hullcam.part.partInfo?.name ?? "");
                         _cameras.Add(new KerbcastCamera(
                             hullcam,

docs/TROUBLESHOOTING.md

@@ -109,5 +109,13 @@ leaves your own game view untouched.
 
 ## Still stuck?
 
-Open an issue at <https://github.com/jonpepler/kerbcast/issues> with the
-`[Kerbcast]`/`[Kerbcast.sidecar]` slice of KSP.log and your OS + KSP version.
+1. Turn on verbose logging: uncomment `DebugCameraLogging = true` in
+   `GameData/Kerbcast/PluginData/settings.cfg` (see
+   [INSTALL.md](INSTALL.md#configuration) for where per-camera settings
+   live).
+2. Reproduce the problem, then quit KSP so the log file is flushed.
+3. Open an issue at <https://github.com/jonpepler/kerbcast/issues> with:
+   - the `[Kerbcast]`/`[Kerbcast.sidecar]` slice of KSP.log
+   - your OS + KSP version
+   - which vessel/camera part was affected, and what you were doing right
+     before it happened (vessel switch, revert, scene change, …)

sidecar/src/cameras.rs

@@ -27,6 +27,29 @@ use crate::encoder::{EncoderBackend, SessionHealth};
 use crate::protocol::{CameraLifecycle, CameraState as ProtocolCameraState, Layer, QualityPreset};
 use crate::shared_mem::{ControlBlock, MmapFrameRing, MmapRingConfig};
 
+/// Filesystem identity for a `.ring` file: distinguishes "same path, same
+/// underlying file" from "same path, file was unlinked and a new one
+/// created in its place". Inode is exact on unix; `MetadataExt::file_index`
+/// would give the same on Windows but is still unstable
+/// (`windows_by_handle`, rust-lang/rust#63010), so non-unix falls back to
+/// the mtime in nanoseconds — coarser (a same-tick delete+recreate could in
+/// theory collide) but still a large improvement over never detecting the
+/// swap at all.
+#[cfg(unix)]
+fn ring_file_identity(meta: &std::fs::Metadata) -> u64 {
+    use std::os::unix::fs::MetadataExt;
+    meta.ino()
+}
+
+#[cfg(not(unix))]
+fn ring_file_identity(meta: &std::fs::Metadata) -> u64 {
+    meta.modified()
+        .ok()
+        .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok())
+        .map(|d| d.as_nanos() as u64)
+        .unwrap_or(0)
+}
+
 /// On-disk shape of `global.status.json` — the plugin → sidecar push
 /// half of the IPC. The plugin rewrites this file at ~1Hz with the
 /// current effective state for every tracked camera plus the global
@@ -255,6 +278,16 @@ struct InfoManifest {
 pub struct CameraState {
     pub flight_id: u32,
     pub ring: MmapFrameRing,
+    /// Filesystem identity (inode on unix, file index on windows) of the
+    /// `.ring` file this entry attached to. `RebuildCameraList` on the
+    /// plugin side can delete and recreate a ring at the same path within
+    /// one Unity frame for a camera whose part persists across a vessel
+    /// switch, without writing a `destroyed` tombstone (that's reserved for
+    /// actual part destruction). A same-second delete+recreate can share an
+    /// mtime, so identity — not mtime — is what `rescan` compares to notice
+    /// the file underneath changed and reattach instead of leaving the mmap
+    /// pointed at the unlinked original.
+    pub ring_identity: u64,
     pub max_width: u32,
     pub max_height: u32,
     pub part_name: String,
@@ -730,14 +763,35 @@ impl CameraRegistry {
         // no ring file, so they won't appear in `found`).
         let mut attached: Vec<u32> = Vec::new();
         for (id, path) in &found {
+            let found_identity = match tokio::fs::metadata(path).await {
+                Ok(meta) => ring_file_identity(&meta),
+                Err(_) => 0,
+            };
             // A ring that reappears for a flight_id we already hold as a
             // destroyed tombstone means KSP reused that part.flightID for a
             // freshly spawned camera (it recycles ids across revert/recover),
             // so fall through and resurrect it: the insert below replaces the
             // tombstone with a fresh active entry and the attach is announced
-            // to peers so their tiles rebind. A live entry is left untouched.
+            // to peers so their tiles rebind. A live entry with an unchanged
+            // ring file identity is left untouched.
+            //
+            // A live (non-destroyed) entry whose ring identity *has* changed
+            // means the plugin deleted and recreated the ring at this path
+            // without a destroyed tombstone — e.g. RebuildCameraList tearing
+            // down and rebuilding a camera on `onVesselChange` whose part
+            // persisted across the switch. Without this check the old mmap
+            // stays pointed at the unlinked file and the feed goes stale
+            // forever, since nothing else in the registry ever changes to
+            // signal a reattach is needed.
             let resurrecting = match cameras.get(id) {
                 Some(existing) if existing.destroyed.load(Ordering::Acquire) => true,
+                Some(existing) if existing.ring_identity != found_identity => {
+                    info!(
+                        flight_id = id,
+                        "ring file identity changed with no destroyed tombstone — reattaching"
+                    );
+                    true
+                }
                 Some(_) => continue,
                 None => false,
             };
@@ -759,6 +813,7 @@ impl CameraRegistry {
                         Arc::new(CameraState {
                             flight_id: *id,
                             ring,
+                            ring_identity: found_identity,
                             max_width: self.ring_cfg.max_width,
                             max_height: self.ring_cfg.max_height,
                             part_name: manifest.part_name,
@@ -1547,6 +1602,67 @@ mod tests {
         );
     }
 
+    // Reproduces the vessel-switch stale-feed bug (kerbcast#2): a camera
+    // whose part persists across `onVesselChange` gets torn down and rebuilt
+    // by RebuildCameraList without a destroyed tombstone (that path is
+    // reserved for actual part destruction), deleting and recreating the
+    // ring at the same flight_id within one frame. Before the ring_identity
+    // check this was silently ignored by rescan (the entry wasn't flagged
+    // destroyed) and the old mmap stayed pointed at the unlinked file
+    // forever — no browser refresh could recover it.
+    //
+    // unix-only: the identity check is exact there (inode). The non-unix
+    // fallback (mtime) can't distinguish two files created back-to-back
+    // within the same tick, which is exactly what this test does — that's
+    // an inherent limit of the tier-2 fallback, not something to assert on.
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn live_camera_reattaches_when_ring_recreated_without_destroyed_tombstone() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let cfg = MmapRingConfig {
+            slot_count: 4,
+            max_width: 64,
+            max_height: 64,
+        };
+        let shm = dir.path();
+        let id: u32 = 7;
+        let write_manifest = || {
+            let content = format!(
+                r#"{{"lifecycle":"active","flight_id":{id},"part_name":"hc.navcam","part_title":"NavCam","camera_name":"NavCam","vessel_name":"booster","supports_zoom":false,"fov":60.0,"fov_min":10.0,"fov_max":90.0,"supports_pan":false,"pan_yaw_min":0.0,"pan_yaw_max":0.0,"pan_pitch_min":0.0,"pan_pitch_max":0.0}}"#,
+            );
+            std::fs::write(shm.join(format!("{id}.info.json")), content).unwrap();
+        };
+        let ring_path = shm.join(format!("{id}.ring"));
+        let registry = CameraRegistry::new(shm.to_path_buf(), cfg);
+
+        // 1. Fresh camera attaches active.
+        MmapFrameRing::create(&ring_path, cfg).expect("create ring");
+        write_manifest();
+        registry.rescan().await;
+        let first = registry.get(id).await.expect("camera attached");
+        assert!(!first.destroyed.load(Ordering::Acquire));
+
+        // 2. Vessel switch: plugin's normal (non-destroyed) teardown deletes
+        // the ring and info.json with no tombstone, then RebuildCameraList
+        // immediately recreates the same flight_id — all within one frame,
+        // well before the sidecar's next ~1Hz rescan tick.
+        std::fs::remove_file(&ring_path).unwrap();
+        std::fs::remove_file(shm.join(format!("{id}.info.json"))).unwrap();
+        MmapFrameRing::create(&ring_path, cfg).expect("recreate ring");
+        write_manifest();
+
+        let outcome = registry.rescan().await;
+        assert!(
+            outcome.attached.contains(&id),
+            "reattach must be announced so peers rebind and stop reading the stale mmap"
+        );
+        let second = registry.get(id).await.expect("camera still present");
+        assert!(
+            !std::sync::Arc::ptr_eq(&first, &second),
+            "the registry entry must be replaced, not left pointing at the unlinked ring"
+        );
+    }
+
     // The force-render profiling override keeps a peerless camera subscribed
     // (so the plugin renders + reports telemetry), and clearing it releases the
     // camera again — the cleanup path POST /profile/render?on=false relies on.