Skip to content

build: fix unified water#1763

Merged
alandtse merged 1 commit into
community-shaders:devfrom
alandtse:unified_water_build
Jan 25, 2026
Merged

build: fix unified water#1763
alandtse merged 1 commit into
community-shaders:devfrom
alandtse:unified_water_build

Conversation

@alandtse
Copy link
Copy Markdown
Collaborator

@alandtse alandtse commented Jan 25, 2026

Renamed members to match commonlib 4.0.0.

Summary by CodeRabbit

  • Refactor
    • Reorganized water system coordinate management to use base cell positioning for consistency.
    • Updated water system API and object handling for improved internal organization.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings January 25, 2026 20:43
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 25, 2026

📝 Walkthrough

Walkthrough

This change refactors the water system API in UnifiedWater by renaming methods (InitializeWater→AddWater, RemoveGeometry→RemoveWater) and standardizing coordinate references from per-node coordinates to base cell coordinates throughout water cache lookups and positional calculations.

Changes

Cohort / File(s) Summary
Water System API Refactoring
src/Features/UnifiedWater.cpp
Updated TESWaterSystem method calls: InitializeWater()AddWater() and RemoveGeometry()RemoveWater(). Switched coordinate system from node-relative (x, y) to base cell coordinates (baseCellX, baseCellY) for WaterCache lookups and positional calculations. Updated corresponding log messages.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested reviewers

  • doodlum

Poem

🐰 With clearer names the waters flow,
From node coordinates, now let them go,
Base cells guide each drop with grace,
AddWater paints the system's face! 💧✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
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.
Title check ❓ Inconclusive The title 'build: fix unified water' is vague and uses generic terms like 'fix' without specifying the nature of the issue being addressed. Provide a more specific title that describes the actual changes, such as 'refactor: update water system API calls and coordinate handling' or 'fix: correct water object lifecycle and coordinate management in unified water system'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches
  • 📝 Generate docstrings

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

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

@github-actions
Copy link
Copy Markdown

Using provided base ref: d9f42f5
Using base ref: d9f42f5
Base commit date: 2026-01-25T03:40:57-08:00 (Sunday, January 25, 2026 03:40 AM)
No actionable suggestions for changed features.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts Unified Water’s terrain-block attach logic to correctly locate per-chunk instructions and to use the appropriate TESWaterSystem APIs when removing/adding water objects.

Changes:

  • Switches water object removal/creation from RemoveGeometry/InitializeWater to RemoveWater/AddWater.
  • Uses BGSTerrainNode::baseCellX/baseCellY (instead of x/y) for instruction lookup, logging, and placement offset math.

@github-actions
Copy link
Copy Markdown

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

@alandtse alandtse merged commit a22883c into community-shaders:dev Jan 25, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants