fix: improve tables and table use#1174
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
6 issues found across 4 files
Confidence score: 2/5
- I’m scoring this as high risk because several medium-high severity findings are concrete runtime risks, not just style/perf concerns (sev 6–7 with strong confidence).
- The most severe functional issue is in
scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml: the right-click mission path can callgroup_selectionwith undefined_unit_l, which can trigger immediate errors when a target unit is missing. scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gmlalso has an emptycatchindraw()(exceptions get silently swallowed) and refresh logic keyed only to fleet count, increasing the chance of hidden failures and stale UI state.- Pay close attention to
scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gmlandscripts/scr_Table/scr_Table.gml- undefined variable flow, unsaferow_method/struct handling, and heavy draw-path work are the main stability and performance risks.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_Table/scr_Table.gml">
<violation number="1" location="scripts/scr_Table/scr_Table.gml:127">
P2: Custom agent: **Code Quality Review**
Heavy per-cell text scaling logic added inside Draw function</violation>
<violation number="2" location="scripts/scr_Table/scr_Table.gml:139">
P1: Guard `row_method` call to struct rows only. Current call path can pass array rows into `struct_exists`, which expects struct.</violation>
</file>
<file name="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml">
<violation number="1" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:160">
P1: Custom agent: **Code Quality Review**
Heavy logic (loop, struct allocations, method bindings) inside draw path: `update_fleet_table()` rebuilds row structs and binds methods from `draw_fleet_area()`, which is called by `inside_method` during `main_panel.draw()`. Move to a Step event or data-change callback.</violation>
<violation number="2" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:233">
P2: Fleet table refresh tied only to fleet count. Data goes stale when fleet state changes but count stays same.</violation>
<violation number="3" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:342">
P1: Right-click mission action can call `group_selection` with undefined `_unit_l` when target unit missing.</violation>
<violation number="4" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:554">
P1: Custom agent: **Code Quality Review**
Empty catch block in `draw()` silently swallows all exceptions without fail-loud logging or rethrowing, violating the codebase's established `handle_exception` pattern.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. This is a substantial refactor of the fleet/mission panels and the underlying Table component, affecting multiple game objects and UI interactions, which warrants manual review to ensure no regressions or logic errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose and Description
Testing done
Related things and/or additional context
Summary by cubic
Rebuilt the Fleets and Missions panels to use the shared
Tablewith row hover/clicks, hover highlight, and per-cell text scaling. Lists are consistent, clickable, and readable; fleets initialize at game start and auto-refresh.New Features
Table: centralized row hover/left/right click viarow_method, hover highlight, per-cell text scaling, min row height,colour/font, androw_count().update_fleet_table()/draw_fleet_area()render viaTable; left-click pans to fleet; hover shows forge/apothecary breakdown on a detail slate; auto-refresh when fleet count changes.mission_tablerenders viaTable; left-click pans to system; integratesobj_en_fleet.events(Deliver Trophy Guard) with hover tooltip; right-click selects the trophy-bearing marine.Refactors
Table; update tables when tabs open; wrapped panel draw in a try-catch.location_viewer.update_fleet_table(); addedeventstoobj_en_fleetfor mission UI data.Written for commit 04edfd8. Summary will update on new commits.