Flyby sequence timeline control

[Lwmte]

Things that need testing:

• Camera inserting at the end of the sequence (playhead at the last camera exactly)
• Camera inserting after the end of the sequence at a specific playhead position
• Camera inserting between two other cameras
• Camera inserting at the start of the sequence (playhead at 0.0)
• Camera freezetime and whether the sequence still aligns with the last camera node on the timeline
• Camera cuts
• ALT + Drag camera reordering
• Zooming in and out
• "Fit to viewport" with mouse double click anywhere
• Viewport panning with SHIFT + Scroll and Middle / Right mouse drag
• Camera multiselect and sync with 3D view
• Flyby form camera tweaking and undo / redo
• Undo / Redo on timeline actions
• TEN camera pause ease-in-out and whether the sequence still aligns with the last camera node on the timeline
• Static camera preview with F8 key (CAMERA_TARGET is determined via a valid legacy trigger setup, or Lara as fallback)

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 2 comments.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

Comments suppressed due to low confidence (4)

TombLib/TombLib/Utils/MathC.cs:1

• Renaming a public constant from ZeroTolerance to Epsilon is a breaking change for any callers referencing MathC.ZeroTolerance. Consider keeping ZeroTolerance as a backwards-compatible alias (optionally marked [Obsolete]) that forwards to Epsilon so older code continues to compile.
  TombEditor/Editor.cs:1
• GetRoomAtPosition() returns null but the method signature returns non-nullable Room. This can propagate unexpected nulls to callers. Change the return type to Room? (and update callers accordingly) or guarantee a Room return (e.g., by throwing or returning a fallback room).
  TombEditor/Forms/FormMain.cs:1
• The comment says 'Disable all hotkeys' during preview, but the code forwards non-PreviewCamera keys to base.ProcessCmdKey(), which can still trigger accelerators/commands. If the intent is to block inputs during preview, swallow the key (return true) when it isn't Escape or PreviewCamera.
  TombEditor/Forms/FormFlybyCamera.cs:1
• HasPendingChanges compares float-cast NumericUpDown values directly to stored floats. Decimal->float conversions can introduce tiny representation differences, causing false positives and unnecessary undo entries. Prefer comparing decimals (store original values as decimals) or compare floats with a small tolerance (e.g., Math.Abs(a-b) <= epsilon).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

Comments suppressed due to low confidence (3)

TombLib/TombLib/Utils/MathC.cs:1

• Renaming the public constant ZeroTolerance to Epsilon is a breaking API change for any external consumers. Consider reintroducing ZeroTolerance as an alias (optionally marked [Obsolete]) that forwards to Epsilon, so existing callers keep compiling while new code migrates.
  TombLib/TombLib/Utils/MathC.cs:1
• Renaming the public constant ZeroTolerance to Epsilon is a breaking API change for any external consumers. Consider reintroducing ZeroTolerance as an alias (optionally marked [Obsolete]) that forwards to Epsilon, so existing callers keep compiling while new code migrates.
  TombEditor/TombEditor.csproj:1
• Pinning LangVersion to C# 12 can break builds for contributors/CI using older .NET SDKs (even if the target framework remains net6.0-windows). If C# 12 features are required, add/adjust a global.json (or documented build requirement) to ensure a compatible SDK is used; otherwise consider removing the explicit LangVersion.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 2 comments.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

Comments suppressed due to low confidence (5)

TombEditor/EditorActions.cs:1

• _editor.ObjectChange(instance, ObjectChangeType.Change); is executed even when formFlyby.HasChanges is false. That will mark the object/level as changed (and potentially dirty the project) even if the user clicked OK without modifying anything. Make ObjectChange(...) conditional on HasChanges (or compute HasChanges in the action layer) so no-op edits don’t raise change events.
  TombEditor/Forms/FormMain.cs:1
• Indexing UI_Hotkeys[\"PreviewCamera\"] can throw KeyNotFoundException for existing user configurations created before this key existed (or if the hotkey set is corrupted). Prefer a safe lookup (TryGetValue) with a sensible fallback (e.g., allow ESC only, or treat as not allowed) to avoid crashing while in preview mode.
  TombEditor/TombEditor.csproj:1
• Setting LangVersion to 12 makes the build depend on a .NET SDK/newer compiler that supports C# 12. If CI/dev environments still use a .NET 6 SDK, builds will fail even though TargetFramework is net6.0-windows. Consider adding a global.json (pinning an SDK that supports C# 12, e.g. .NET 8+) and/or updating build documentation/CI to ensure consistent tooling.
  TombEditor/Forms/FormFlybyCamera.cs:1
• numFOV.Value, numRoll.Value, and rotation numeric assignments cast directly from float to decimal without clamping/validation. If any of these values are outside the NumericUpDown min/max (or are NaN/Infinity), WinForms will throw when setting Value, which can prevent the dialog from opening. Apply the same ClampNumericValue(...) approach to FOV/Roll/RotationX/RotationY as you did for Speed.
  TombEditor/Forms/FormFlybyCamera.cs:1
• Casting a large finite float to decimal can throw OverflowException before clamping (line 203). To make this helper robust to out-of-range inputs, clamp in floating-point space first (using bounds converted to float/double) or guard the cast with a safe range check/try-catch, then clamp as decimal.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 2 comments.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

Comments suppressed due to low confidence (3)

TombLib/TombLib/Utils/MathC.cs:1

• Renaming the public constant ZeroTolerance to Epsilon is a breaking API change for any downstream code that references MathC.ZeroTolerance. To preserve compatibility, keep ZeroTolerance as an alias (optionally marked [Obsolete]) that forwards to Epsilon, or update all public callers across the repo in the same PR.
  TombLib/TombLib/Utils/MathC.cs:1
• Renaming the public constant ZeroTolerance to Epsilon is a breaking API change for any downstream code that references MathC.ZeroTolerance. To preserve compatibility, keep ZeroTolerance as an alias (optionally marked [Obsolete]) that forwards to Epsilon, or update all public callers across the repo in the same PR.
  TombEditor/Forms/FormFlybyCamera.cs:1
• When the dialog is cancelled and _ownedPreview is false (preview was started elsewhere), RestoreOriginalValues() reverts the camera but the active preview may remain showing the last live-updated parameters. Consider issuing a preview refresh after restore (e.g., call the preview update event method) when preview is still active, so the viewport matches the restored camera state.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 3 comments.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

Comments suppressed due to low confidence (2)

TombLib/TombLib/LevelData/Instances/FlybyCameraInstance.cs:1

• Right-shifting a signed short via (int)Timer >> 4 will sign-extend when bit 15 is set, producing negative frame counts for values >= 0x8000. If Timer is meant to be treated as an unsigned 16-bit payload (as flyby timers often are), cast to ushort before shifting (e.g., (ushort)Timer) so decoding is consistent for the full range.
  TombEditor/TombEditor.csproj:1
• Setting <LangVersion>12</LangVersion> while targeting net6.0-windows can break builds in environments still using .NET 6/7 SDKs (C# 12 ships with .NET 8 tooling). If the repo isn't pinned to a .NET SDK that supports C# 12 (e.g., via global.json), consider either pinning the SDK accordingly or using a compatible language version to avoid CI/dev toolchain failures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated no new comments.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

Comments suppressed due to low confidence (10)

TombLib/TombLib/Utils/MathC.cs:1

• Renaming the public constant ZeroTolerance to Epsilon is a breaking change for any external consumers of TombLib. If TombLib is used outside this repo, consider keeping ZeroTolerance as a backward-compatible alias (e.g., public const float ZeroTolerance = Epsilon;) and optionally marking it obsolete so downstream code doesn’t break.
  TombLib/TombLib/LevelData/Instances/ObjectGroup.cs:1
• With the new stable _rootObject field, Clone() currently rebuilds the group from a HashSet enumeration, which is not deterministic. This can silently change which object becomes the root in the cloned group (because the list passed to ObjectGroup(IReadOnlyList<...>) picks the first element as root). To preserve behavior, build the clone list so the cloned root corresponds to the original _rootObject first, then add the remaining clones.
  TombLib/TombLib/LevelData/Instances/ObjectGroup.cs:1
• With the new stable _rootObject field, Clone() currently rebuilds the group from a HashSet enumeration, which is not deterministic. This can silently change which object becomes the root in the cloned group (because the list passed to ObjectGroup(IReadOnlyList<...>) picks the first element as root). To preserve behavior, build the clone list so the cloned root corresponds to the original _rootObject first, then add the remaining clones.
  TombEditor/ToolWindows/MainView.cs:1
• This replaces panelStats.Visible = settings.UI_ShowStats; with tbStats.Visible = .... If panelStats is still the actual statistics panel (and tbStats is only a toolbar item), the user setting may no longer control stats panel visibility. Consider restoring the previous panelStats.Visible assignment (or updating both toolbar and panel as intended) and keep UpdateBottomPanelVisibility() based on the panels that actually affect layout.
  TombEditor/Forms/FormFlybyCamera.cs:1
• The form now mutates _flyByCamera during live preview (PreviewParameter_Changed). On Cancel, values are restored, but the viewport preview is not updated to reflect the restored state (and no editor change notification is raised). This can leave the preview camera showing the last edited frame even though the underlying instance was reverted. Consider updating the preview after restore (e.g., call the same preview update pipeline used during edits) and/or avoiding direct mutation of the model during preview by previewing from a temporary frame/state instead.
  TombEditor/Forms/FormFlybyCamera.cs:1
• The form now mutates _flyByCamera during live preview (PreviewParameter_Changed). On Cancel, values are restored, but the viewport preview is not updated to reflect the restored state (and no editor change notification is raised). This can leave the preview camera showing the last edited frame even though the underlying instance was reverted. Consider updating the preview after restore (e.g., call the same preview update pipeline used during edits) and/or avoiding direct mutation of the model during preview by previewing from a temporary frame/state instead.
  TombLib/TombLib/LevelData/Instances/FlybyCameraInstance.cs:1
• There are now multiple definitions of the same flyby max speed constant (MaxFlybySpeed here and FlybyConstants.MaxSpeed elsewhere). That increases the chance of drift. Prefer centralizing the value in one shared location (e.g., expose it from TombLib where FlybyCameraInstance lives and reference that from editor/UI code), or document why duplication is required due to assembly boundaries.
  TombLib/TombLib/LevelData/Instances/FlybyCameraInstance.cs:1
• There are now multiple definitions of the same flyby max speed constant (MaxFlybySpeed here and FlybyConstants.MaxSpeed elsewhere). That increases the chance of drift. Prefer centralizing the value in one shared location (e.g., expose it from TombLib where FlybyCameraInstance lives and reference that from editor/UI code), or document why duplication is required due to assembly boundaries.
  TombLib/TombLib/Utils/CatmullRomSpline.cs:1
• This introduces new spline evaluation code that directly affects rendered flyby paths (Panel3D now uses CatmullRomSpline.EvaluatePositions). The PR adds good flyby timing/cache tests, but there are no unit tests validating the spline evaluator itself (e.g., that it hits control points exactly at integer t, and that EvaluatePositions returns the final control point). Adding a small test suite for CatmullRomSpline would help prevent subtle path regressions.
  TombLib/TombLib/Utils/CatmullRomSpline.cs:1
• This introduces new spline evaluation code that directly affects rendered flyby paths (Panel3D now uses CatmullRomSpline.EvaluatePositions). The PR adds good flyby timing/cache tests, but there are no unit tests validating the spline evaluator itself (e.g., that it hits control points exactly at integer t, and that EvaluatePositions returns the final control point). Adding a small test suite for CatmullRomSpline would help prevent subtle path regressions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Nickelony]

Added testing checklist to OP.

[Copilot]

Pull request overview

Copilot reviewed 54 out of 59 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

• TombEditor/Forms/FormMain.Designer.cs: Language not supported
• TombEditor/ToolWindows/MainView.Designer.cs: Language not supported

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.