fix: adjust IsInterior to consider kNoSky or kFixedDimensions flags

[doodlum]

Summary by CodeRabbit

• New Features

  • None.

• Bug Fixes

  • Improved detection of interior spaces, including fixed-dimension areas, reducing misclassified locations.
  • Enhances ambience, audio, lighting, and weather handling in affected areas for more consistent behavior.
  • Improves loading behavior, map markers, and fast-travel restrictions where locations should function as interiors.

• Chores

  • Internal logic updated with no changes to public APIs.

[coderabbitai]

Walkthrough

Adjusts IsInterior() logic in src/Utils/Game.cpp to consider a cell interior if either kNoSky or kFixedDimensions is set, instead of requiring all worldSpace flags to be kNoSky. No public interfaces changed.

Changes

Cohort / File(s)	Summary
Interior detection logic src/Utils/Game.cpp	Modified IsInterior() condition: now returns true if world space has kNoSky OR kFixedDimensions, instead of requiring only kNoSky. No signature changes; control flow remains an early-return check with updated predicate.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller
    participant Game as Game::IsInterior()
    participant TES as WorldSpace Flags

    Caller->>Game: IsInterior(cell)
    Game->>TES: Read worldSpace flags
    alt New condition
        Note right of Game: Interior if (kNoSky OR kFixedDimensions)
        TES-->>Game: flags
        Game-->>Caller: true/false
    else Old condition
        Note right of Game: Interior if (kNoSky only)
        TES-->>Game: flags
        Game-->>Caller: true/false
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: fix some internal errors when debugging #1500 — Also modifies IsInterior() in src/Utils/Game.cpp, affecting the same function.
• feat(VR): separate interior/exterior options for depth buffer culling #1362 — Changes IsInterior() logic impacting VR::EarlyPrepass() interior/exterior handling.
• fix: misc fixes #1474 — Adjusts interior detection via sky-mode checks, related to IsInterior() behavior.

Suggested reviewers

• alandtse

Poem

I hop through flags beneath the sky,
A bitwise wink, an OR hopped by—
No roof? Fixed bounds? I’ll call it in,
Interior tales now may begin.
Thump-thump, review, a tidy spin. 🐇✨

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 succinctly and accurately summarizes the core change: modifying IsInterior to treat kNoSky or kFixedDimensions as interior indicators; it is specific to the function and flags and avoids vague wording or extraneous details.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-interior-detection

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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]

Using provided base ref: b3129e2
Using base ref: b3129e2
Base commit date: 2025-09-22T21:30:57+01:00 (Monday, September 22, 2025 09:30 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Utils/Game.cpp (1)

169-171: Bug: dynamic resolution not applied (return value ignored).

ConvertToDynamic returns the adjusted size but its result isn’t assigned, so dispatch counts stay at full resolution even when a_dynamic is true.

Apply:

-        if (a_dynamic)
-            ConvertToDynamic(resolution);
+        if (a_dynamic)
+            resolution = ConvertToDynamic(resolution);

🧹 Nitpick comments (1)

src/Utils/Game.cpp (1)

12-17: Comment mismatch (typos) in StoreTransform3x4NoScale doc block.

The Y/Z lines in the comment repeat .x. The code is correct; just fix the comment to avoid confusion.

Proposed edit:

-// Dest[1][3] = Source.m_Translate.x;
+// Dest[1][3] = Source.m_Translate.y;
-// Dest[2][3] = Source.m_Translate.x;
+// Dest[2][3] = Source.m_Translate.z;

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3129e2 and a7495b2.

📒 Files selected for processing (1)

• src/Utils/Game.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

• src/Utils/Game.cpp

src/**/*.{cpp,cxx,cc,h,hpp,hxx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code

Files:

• src/Utils/Game.cpp

🧠 Learnings (1)

📓 Common learnings

Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.

Learnt from: ThePagi
PR: doodlum/skyrim-community-shaders#1369
File: package/Shaders/Lighting.hlsl:0-0
Timestamp: 2025-08-05T17:40:44.828Z
Learning: In the skyrim-community-shaders repository, ultra trees (object LOD trees) are detected using a compound shader define condition `defined(DO_ALPHA_TEST) && defined(LOD_BLENDING) && defined(RIM_LIGHTING) && defined(SOFT_LIGHTING)` because "they have no define" according to the comment. The `ExtraFlags::IsTree` flag is used for different tree handling (AO removal in skylighting) and may not apply to ultra trees specifically. Before replacing the compound condition with `IsTree`, verification is needed to ensure the flag covers ultra trees.

Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build plugin and addons

🔇 Additional comments (1)

src/Utils/Game.cpp (1)

308-311: Double-check broadening interior detection to include kFixedDimensions — verification required

Automated verification failed: src/Utils/Game.cpp was not present in the checkout and ripgrep skipped files (unrecognized file type 'cc'), so call sites and affected worldspaces could not be confirmed.

• If intent is “interior worldspaces,” use kNoSky only (safest).
• If keeping kFixedDimensions, gate it behind a setting/whitelist and add telemetry to confirm which worldspaces are affected (check Sovngarde, Soul Cairn, Apocrypha, Blackreach).
• Return false when worldSpace == nullptr instead of falling through to true.

Re-run locally from the repo root to list call sites:

• rg -nC3 '\bIsInterior\s*(' -S --hidden
  or
• git grep -n "IsInterior"

If those return nothing, attach src/Utils/Game.cpp (≈lines 280–340) or the correct file path so I can re-verify.

[github-actions]

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