bugfix(w3d): Skip rivers that overflow the dynamic index buffer

[tintinhamans]

• Follow up for bugfix(render2d): Flush batched 2D draws before they overflow the dynamic buffers #3487

drawRiverWater asks DynamicIBAccessClass for three indices per river point, and the count is an unsigned short. 21846 points wrap to 2, and the loop still writes 65532 indices.

The point count comes straight from the map file, so one river polygon is enough to write up to 128 KB past the index buffer.

Rivers that do not fit in 16 bits are now skipped. The largest river in the 537 retail and custom maps I tested has 374 points.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 27318f3f-4be1-4d66-9296-795dde1614b7

---

📥 Commits

Reviewing files that changed from the base of the PR and between 05fe7ce and 600358d.

---

📒 Files selected for processing (1)

• Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp

---

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8ca882d2-a0a5-45ba-b7c6-38e04f07f327

---

---

📥 Commits

Reviewing files that changed from the base of the PR and between 0e6a06c and 05fe7ce.

---

---

📒 Files selected for processing (1)

• Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp

---

---

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

---

---

Walkthrough

The river water draw path now computes the index count before allocating the dynamic index buffer. It returns without drawing when the count exceeds 65,535. Rivers within the limit retain the existing rendering path.

Changes

River water rendering

Layer / File(s)	Summary
Check river index count Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp	drawRiverWater returns when the computed index count exceeds 65,535. The check runs before m_drawingRiver is set or the dynamic index buffer is allocated.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: bobtista

Merge Risk: ⚪ Minimal · up to 05fe7

Oversized rivers are skipped before the index buffer is allocated, preventing the reported count-wrap path. No merge-blocking issue is evident from the supplied change context.

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check		The description clearly explains the overflow risk, the fix, and the test scope.
Title check		The title clearly and concisely describes the main change: skipping rivers that overflow the dynamic index buffer.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge, with no outstanding findings.

Summary

The PR skips rivers whose requested index count exceeds the 16-bit buffer limit.

• Oversized rivers are skipped before their index buffers are written.

_{Reviews (2) · Last reviewed commit: "bugfix(w3d): Skip rivers that overflow t..." · Reviewed by Greptile}

[macroscopeapp]

🟡 Medium Water/W3DWater.cpp:2743

Rivers with 21,846 points are skipped even though they generate only 65,532 indices, which fit in the 16-bit buffer. indexCount includes an extra quad, evaluates to 65,538, and triggers the early return; calculate it from the rectangleCount * 6 indices the loop writes.

Suggested change

	const Int indexCount = (rectangleCount+1)*2*3;
	const Int indexCount = rectangleCount*2*3;

🤖 Copy this AI Prompt to have your agent fix this:

In file @Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp around line 2743:

Rivers with 21,846 points are skipped even though they generate only 65,532 indices, which fit in the 16-bit buffer. `indexCount` includes an extra quad, evaluates to 65,538, and triggers the early return; calculate it from the `rectangleCount * 6` indices the loop writes.

[tintinhamans]

The check mirrors the count requested from DynamicIBAccessClass, which is what wraps.

[macroscopeapp]

The request should match the loop’s rectangleCount * 6 writes. At 21,846 points that is 65,532, which fits; the current extra six causes an unnecessary skip. Counts above that still return before the unsigned short constructor can wrap.

[tintinhamans]

Matching the loop gains two points and makes a 3 point river lock zero indices.

[macroscopeapp]

You're right—the direct replacement would zero-lock a 3-point river. The count needs to retain a six-index minimum while using rectangleCount * 6 for nonzero rectangles.