Fix group-only query bug and improve filtering consistency#5
Fix group-only query bug and improve filtering consistency#5Donach wants to merge 1 commit intognet/fix_examplefrom
Conversation
- Fixed QueryBuilder.is_empty() to account for groups and enabled filter - Fixed QueryBuilder.matches() to correctly filter by groups and enabled filter - Updated System.gd's _query_has_non_structural_filters() and _filter_entities_global() to include enabled filter check for consistency - Verified that group filtering logic was already correctly implemented in System.gd but was being bypassed due to inconsistent state reporting in QueryBuilder Co-authored-by: Donach <39565367+Donach@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Code Review — APPROVEDReviewer: Codex (automated) SummaryReviewed against I did not find a correctness issue in the patch. The repo already contains focused coverage for the affected behavior in Issues FoundNone. Verdict✅ APPROVED — ready for squash-merge |
2 participants
This change fixes a bug where querying for a nonexistent group returned all entities instead of zero. The root cause was that
QueryBuilder.is_empty()did not account for group filters, causing some parts of the framework to treat group-only queries as "empty" (match-all). Additionally,QueryBuilder.matches()was missing the group filtering logic, which meant that when it was called (e.g., during system execution or observer matching), it failed to exclude entities not in the required groups.The fix involves:
QueryBuilder.is_empty()to include checks for_groups,_exclude_groups, and_enabled_filter.QueryBuilder.matches()to perform filtering based on these fields.System.gdto ensure the enabled filter is also correctly identified as a non-structural filter requiring the fallback filtering path.These changes ensure that queries with only group or enabled filters are correctly handled and filtered across the entire ECS framework.
PR created automatically by Jules for task 16334715888511181226 started by @Donach