Skip to content

Fix count bug in _get_polygon_count - #1430

Merged
Arctis-Fireblight merged 1 commit into
Redot-Engine:masterfrom
GeneralProtectionFault:poly_lightfix
Sep 21, 2026
Merged

Arctis-Fireblight merged 1 commit into
Redot-Engine:masterfrom
GeneralProtectionFault:poly_lightfix

Conversation

@GeneralProtectionFault

@GeneralProtectionFault GeneralProtectionFault commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

The light occluder returns the vertex count instead of the poly count in the _get_polygon_count function (override).
It looks like this caused the polygon to redraw unnecessarily for each vertex, which also caused the light blue color of the selected vertex not to show up on a LightOccluder2D.

This fixes that, and just returns 1 for the number of polygons (as the base does), and we can see the blue return to the selected vertex.

I despaired at removing the override entirely out of risk aversity :-P.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected 2D light occluder counting so valid polygons are reported as one occluder, while empty or invalid polygons report zero.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: Redot-Engine/redot-engine/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 91d9fa83-f814-411f-803c-c8c75bdcd58a

📥 Commits

Reviewing files that changed from the base of the PR and between 2ab24bc and aebcfb1.

📒 Files selected for processing (1)
  • editor/scene/2d/light_occluder_2d_editor_plugin.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The editor now reports 1 for a valid, non-empty occluder polygon and 0 for invalid or empty polygons. It no longer returns the polygon’s vertex count.

Changes

Occluder polygon count

Layer / File(s) Summary
Update polygon count logic
editor/scene/2d/light_occluder_2d_editor_plugin.cpp
_get_polygon_count() now returns a binary count based on polygon validity and emptiness.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: arctis-fireblight, decryptedchaos

🚥 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 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix to the count bug in _get_polygon_count, which matches the main change.
  • 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

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

@Arctis-Fireblight
Arctis-Fireblight enabled auto-merge (squash) September 21, 2026 02:57
@Arctis-Fireblight
Arctis-Fireblight merged commit 5592dc3 into Redot-Engine:master Sep 21, 2026
19 checks passed
@github-project-automation github-project-automation Bot moved this from Open to Done in Engine Overview Sep 21, 2026
@GeneralProtectionFault
GeneralProtectionFault deleted the poly_lightfix branch September 21, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants