Skip to content

fix: replace eastl vector with std#1672

Closed
doodlum wants to merge 1 commit into
devfrom
eastl-fix
Closed

fix: replace eastl vector with std#1672
doodlum wants to merge 1 commit into
devfrom
eastl-fix

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented Jan 3, 2026

This fixes potential crashes with grass collision and lighting. Unfortunately, newer EASTL versions use a version scheme which is incompatible with vcpkg

Summary by CodeRabbit

  • Refactor
    • Upgraded internal container implementations in grass collision detection and lighting systems. These code improvements enhance codebase consistency and long-term maintainability through modernized library usage and practices. The changes strengthen code quality and developer efficiency without altering gameplay mechanics, visual quality, performance characteristics, or any user-facing features.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

Two C++ source files were modified to replace EASTL (Electronic Arts Standard Template Library) vector containers with standard library std::vector in local variable declarations. The replacements maintain identical functionality with no logic or control flow modifications across the affected scopes.

Changes

Cohort / File(s) Summary
Container Library Migration
src/Features/GrassCollision.cpp, src/Features/LightLimitFix.cpp
Replaced eastl::vector with std::vector in local variable declarations. In GrassCollision::UpdateCollisions: actorList, boundingBoxData, collisionsData, collisionShapes. In LightLimitFix::UpdateLights: LightData accumulation container. No behavioral changes; usage patterns (reserve, push_back) remain identical.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Farewell EASTL, hello std,
Our vectors now wear simpler threads,
Same logic flows, no changes made,
Just standard types in their cascade.
East meets west in harmony! 🎉

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 'fix: replace eastl vector with std' accurately and concisely summarizes the main change in the pull request: replacing EASTL vectors with std::vector across two files (GrassCollision.cpp and LightLimitFix.cpp).
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent 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 5ed6701 and f30541a.

📒 Files selected for processing (2)
  • src/Features/GrassCollision.cpp
  • src/Features/LightLimitFix.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/Features/LightLimitFix.cpp
  • src/Features/GrassCollision.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/Features/LightLimitFix.cpp
  • src/Features/GrassCollision.cpp
**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

  1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
    if the existing title does not describe the code changes):
    Format: type(scope): description
    Length: 50 characters limit for title, 72 for body
    Style: lowercase description, no ending period
    Examples:

    • feat(vr): add cross-eye sampling
    • fix(water): resolve flowmap bug
    • docs: update shader documentation
  2. Issue References (if PR fixes bugs or implements features):
    Suggest adding appropriate GitHub keywords:

Otherwise, use your standard review approach focusing on code quality.

Files:

  • src/Features/LightLimitFix.cpp
  • src/Features/GrassCollision.cpp
🧠 Learnings (1)
📓 Common learnings
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 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
Repo: doodlum/skyrim-community-shaders PR: 1369
File: src/Features/SnowCover.cpp:277-293
Timestamp: 2025-08-05T18:22:40.578Z
Learning: In the skyrim-community-shaders SnowCover feature, the wstrtostr and strtowstr utility functions defined in src/Features/SnowCover.cpp are unused and should be removed rather than fixed, as confirmed by the original author ThePagi.
Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to src/**/*.{cpp,cxx,cc,h,hpp,hxx} : Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
⏰ 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 (2)
src/Features/GrassCollision.cpp (1)

25-25: LGTM! Vector replacements are correct.

The replacement of eastl::vector with std::vector for these local variables is straightforward and correct. All operations used (.reserve(), .push_back(), .begin(), .end(), .data(), .size()) are fully compatible between the two implementations.

Note: The codebase still uses eastl::make_unique (lines 278, 296), which appears to be an intentional partial migration strategy focusing on local containers.

Also applies to: 54-54, 57-57, 72-72

src/Features/LightLimitFix.cpp (1)

387-387: LGTM! Vector replacement is correct.

The replacement of eastl::vector<LightData> with std::vector<LightData> is correct. All operations used (.reserve(), .push_back(), .data(), .size()) are fully compatible, and the change maintains identical functionality.

This aligns with the PR's strategy to replace EASTL vectors while retaining other EASTL utilities like eastl::make_unique elsewhere in the codebase.


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

github-actions Bot commented Jan 3, 2026

Using provided base ref: 5ed6701
Using base ref: 5ed6701
Base commit date: 2025-12-31T14:40:51+10:00 (Wednesday, December 31, 2025 02:40 PM)
No actionable suggestions for changed features.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 3, 2026

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

@alandtse
Copy link
Copy Markdown
Collaborator

alandtse commented Jan 3, 2026

Only 2 files use eastl?

#1673 is probably the better end game fix. We'll need to get it updated in the upstream repo and then we can point to it.

@doodlum doodlum closed this Jan 5, 2026
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.

2 participants