fix(VR): allow for Engine Fixes VR 7.x (NG)

[alandtse]

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of plugin dependency loading so VR-specific and standard variants are attempted with a fallback.
  • Enhanced and centralized error messages when a required plugin fails to load, making missing-dependency reports clearer.
  • Reduced strict-required list so only the crash-reporting plugin remains mandatory; other variants now report failures via the new messaging.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors DLL-loading in src/XSEPlugin.cpp: adds a shared pushMissingDllError helper, removes Engine Fixes from the requiredDLLs array, and implements explicit conditional loading that tries EngineFixesVR.dll first (when VR) and falls back to EngineFixes.dll, logging/appending missing-DLL errors.

Changes

Cohort / File(s)	Summary
DLL loading & error helper src/XSEPlugin.cpp	Added pushMissingDllError helper; removed Engine Fixes from requiredDLLs; changed Load() flow to attempt EngineFixesVR.dll then EngineFixes.dll (VR-aware) and route missing-DLL failures through the new helper. CrashLogger.dll remains in requiredDLLs and its failures also use the helper.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• fix: fix engine fixes requirement on vr #1461 — Modifies EngineFixes vs EngineFixesVR selection and related Load() logic in src/XSEPlugin.cpp.
• chore: configure CS to require engine fixes #1452 — Changes requiredDLLs handling for EngineFixes within src/XSEPlugin.cpp.
• fix(VR): fix Shadowmap Cascade Rasterizer #1888 — Adjusts Load() logic in src/XSEPlugin.cpp, related to VR library/version checks.

Suggested reviewers

• jiayev
• doodlum
• davo0411

Poem

🐰 I hopped through code with nimble cheer,
Two DLLs for paths both far and near.
I tucked a helper in the log,
So missing errors leave no fog.
Hooray — the loader's path is clear! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(VR): allow for Engine Fixes VR 7.x (NG)' directly describes the main change: updating VR support to work with Engine Fixes VR 7.x. This matches the raw summary showing refactored Engine Fixes DLL loading with VR-specific handling.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

No actionable suggestions for changed features.

[coderabbitai]

🧹 Nitpick comments (1)

src/XSEPlugin.cpp (1)

197-210: Optional: unify missing-DLL error construction for consistency.

The VR branch uses a hardcoded message while the non-VR branch uses formatted path text. A tiny helper would reduce duplication and keep future messages aligned.

Suggested cleanup

+	auto pushMissingDllError = [&](std::string_view dllRequirement) {
+		auto errorMessage = std::format("Required DLL {} was missing", dllRequirement);
+		logger::error("{}", errorMessage);
+		errors.push_back(errorMessage);
+	};
+
 	// Engine Fixes: VR accepts either EngineFixesVR.dll or the EngineFixes.dll NG
 	if (REL::Module::IsVR()) {
 		if (!LoadLibrary(L"Data/SKSE/Plugins/EngineFixesVR.dll") && !LoadLibrary(L"Data/SKSE/Plugins/EngineFixes.dll")) {
-			auto errorMessage = std::string("Required DLL EngineFixesVR.dll or EngineFixes.dll was missing");
-			logger::error("{}", errorMessage);
-			errors.push_back(errorMessage);
+			pushMissingDllError("EngineFixesVR.dll or EngineFixes.dll");
 		}
 	} else {
 		if (!LoadLibrary(L"Data/SKSE/Plugins/EngineFixes.dll")) {
-			auto errorMessage = std::format("Required DLL {} was missing", stl::utf16_to_utf8(L"Data/SKSE/Plugins/EngineFixes.dll").value_or("<unicode conversion error>"s));
-			logger::error("{}", errorMessage);
-			errors.push_back(errorMessage);
+			pushMissingDllError(stl::utf16_to_utf8(L"Data/SKSE/Plugins/EngineFixes.dll").value_or("<unicode conversion error>"s));
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/XSEPlugin.cpp` around lines 197 - 210, Unify the missing-DLL error
construction by introducing a small helper (e.g., MakeMissingDllMessage or
formatMissingDllError) and use it in both branches where LoadLibrary is called;
replace the hardcoded VR message and the formatted non-VR message with calls
that produce the same style (use stl::utf16_to_utf8 and std::format inside the
helper) and then pass the result to logger::error and errors.push_back so both
the VR branch (checking EngineFixesVR.dll / EngineFixes.dll) and the non-VR
branch (EngineFixes.dll) produce consistent messages.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/XSEPlugin.cpp`:
- Around line 197-210: Unify the missing-DLL error construction by introducing a
small helper (e.g., MakeMissingDllMessage or formatMissingDllError) and use it
in both branches where LoadLibrary is called; replace the hardcoded VR message
and the formatted non-VR message with calls that produce the same style (use
stl::utf16_to_utf8 and std::format inside the helper) and then pass the result
to logger::error and errors.push_back so both the VR branch (checking
EngineFixesVR.dll / EngineFixes.dll) and the non-VR branch (EngineFixes.dll)
produce consistent messages.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9cb807e0-a707-4748-828b-7a2937fc6350

📥 Commits

Reviewing files that changed from the base of the PR and between d556a95 and 53504ca.

📒 Files selected for processing (1)

• src/XSEPlugin.cpp

[github-actions]

✅ A pre-release build is available for this PR:
Download

[davo0411]

this won't add any weird overhead or annoying maintenance right? Supporting two sets of engine fixes? Will there come a time where we need xyz feature from only the new versions ?

[coderabbitai]

🧹 Nitpick comments (1)

src/XSEPlugin.cpp (1)

203-212: Log which Engine Fixes DLL satisfied the VR check.

The runtime-gated fallback looks right. One small follow-up: emit an info line for the DLL that matched so mixed-install reports show whether VR found EngineFixesVR.dll directly or fell back to EngineFixes.dll.

💡 Possible tweak

  // Engine Fixes: VR accepts either EngineFixesVR.dll or the EngineFixes.dll NG
  if (REL::Module::IsVR()) {
-     if (!LoadLibrary(L"Data/SKSE/Plugins/EngineFixesVR.dll") && !LoadLibrary(L"Data/SKSE/Plugins/EngineFixes.dll")) {
+     if (LoadLibrary(L"Data/SKSE/Plugins/EngineFixesVR.dll")) {
+         logger::info("Detected EngineFixesVR.dll");
+     } else if (LoadLibrary(L"Data/SKSE/Plugins/EngineFixes.dll")) {
+         logger::info("Detected EngineFixes.dll");
+     } else {
          pushMissingDllError("EngineFixesVR.dll or EngineFixes.dll");
      }
  } else {

As per coding guidelines "Ensure code changes work across SE/AE/VR Skyrim variants using runtime detection and `REL::RelocateMember()` patterns for compatibility"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/XSEPlugin.cpp` around lines 203 - 212, The VR/desktop DLL probing block
logs failures but not which DLL actually satisfied the check; update the
REL::Module::IsVR() branch so after a successful LoadLibrary call you emit an
info log indicating which DLL was loaded (e.g., "Loaded EngineFixesVR.dll" or
"Fell back to EngineFixes.dll") using the same logging mechanism as
pushMissingDllError, and likewise emit an info log in the non-VR path when
LoadLibrary(L"Data/SKSE/Plugins/EngineFixes.dll") succeeds; keep the existing
failure calls to pushMissingDllError unchanged and reference the exact DLL names
and functions REL::Module::IsVR(), LoadLibrary, and pushMissingDllError when
making the change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/XSEPlugin.cpp`:
- Around line 203-212: The VR/desktop DLL probing block logs failures but not
which DLL actually satisfied the check; update the REL::Module::IsVR() branch so
after a successful LoadLibrary call you emit an info log indicating which DLL
was loaded (e.g., "Loaded EngineFixesVR.dll" or "Fell back to EngineFixes.dll")
using the same logging mechanism as pushMissingDllError, and likewise emit an
info log in the non-VR path when
LoadLibrary(L"Data/SKSE/Plugins/EngineFixes.dll") succeeds; keep the existing
failure calls to pushMissingDllError unchanged and reference the exact DLL names
and functions REL::Module::IsVR(), LoadLibrary, and pushMissingDllError when
making the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2d6bae6f-7491-431d-8692-74f4bbd15e79

📥 Commits

Reviewing files that changed from the base of the PR and between 53504ca and a935213.

📒 Files selected for processing (1)

• src/XSEPlugin.cpp