Fix Kilt conflict in 1.11.x

[NerjalNosk]

BlockEntityRenderer#getRenderBoundingBox clashes with the base neoforge injection brought in by Kilt when it is present, this aims to avoid this.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: f24023c3-1ac9-4356-9c90-8bcaf981f573

📥 Commits

Reviewing files that changed from the base of the PR and between ae786c8 and fa63cc6.

📒 Files selected for processing (2)

• src/main/java/dev/tr7zw/entityculling/BlockEntityRenderFabricExtension.java
• src/main/java/dev/tr7zw/entityculling/EntityCullingMod.java

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

---

Walkthrough

The Fabric block-entity bounding-box method was renamed to ec$getRenderBoundingBox, and its call site now uses the renamed method.

Changes

Fabric bounding-box method

Layer / File(s)	Summary
Rename method and update call site src/main/java/dev/tr7zw/entityculling/BlockEntityRenderFabricExtension.java, src/main/java/dev/tr7zw/entityculling/EntityCullingMod.java	The default method and its Fabric call site now use the name ec$getRenderBoundingBox. The method’s parameter, return type, and implementation are unchanged.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: tr7zw

Merge Risk: ⚪ Minimal · up to fa63c

No actionable merge-blocking risk is established by the reviewed change. The Fabric renderer lookup and renamed method are aligned.

Security Architecture Review

Security architecture risk: 🔵 Low · up to fa63c

The implementation and internal caller remain aligned, with no identified increase in security exposure. Residual risk concerns compatibility with external integrations and unverified behavior when Kilt is present.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is renderer dispatch and cached block-entity culling bounds within the client. The inspected change does not expand this flow into a service, tenant, persistent store, or privileged resource; runtime interactions with external mods are not established.

Trust Boundaries and Controls

• inferred — The inspected source retains the same block-entity input, renderer lookup, extension cast, and culling eligibility checks. Renaming this dispatch introduces no demonstrated new input channel, authority transfer, or bypass. The eligibility checks govern culling selection, not authentication or authorization.

🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the purpose of the change: fixing the Kilt conflict in the 1.11.x code path.
Description check	✅ Passed	The description directly explains that the method rename avoids a BlockEntityRenderer#getRenderBoundingBox conflict caused by the Kilt NeoForge injection.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

[sonarqubecloud]

Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[tr7zw]

This kinda defeats the point of the method, that mods that are on neoforge and fabric can provide this same method for compatibility. And Kilt/EntityCulling are apparently also not the only mods having this exact default method, I think the best would be to get this also into the Fabric API, so all 3 modloaders support it. Alternatively, we could do some tiny agreed-upon lib mod that just gets bundled?

[NerjalNosk]

Or you could provide a similar wrapper on both loaders, with a unique enough name to avoid such conflicts. Regardless of the method, I only wish to avoid having to rollback this mod this

[tr7zw]

Ofc could make the same wrapper method for all modloaders, but that doesn't solve the core issue. Then mods need to explicitly add support for EntityCulling with duplicate methods, and Kilt remains incompatible with other mods implementing this interface. There has to be a cleaner solution. (Also cleaner than skipping the mixin when kilt is loaded, that will work too, but you need to know in advance which mods are an issue)

[tr7zw]

Replaced by the other PR.