refactor: More stray variable definition and loop tune ups.#1250
refactor: More stray variable definition and loop tune ups.#1250carys-the-weed-cloud wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
5 issues found across 45 files
Confidence score: 3/5
- In
scripts/scr_Table/scr_Table.gml, removing_heading = headings[i];after converting strings toReactiveStringcan leave_headingas a plain string, so later.w/.update()calls may fail at runtime and break table heading rendering—restore the reassignment (or otherwise guarantee_headingis the reactive object) before merging. - In
scripts/scr_crusade/scr_crusade.gml,cleanis initialized as scalar0and then treated like an array while also appearing to be unused, which risks subtle logic mistakes and makes future changes error-prone—initialize it explicitly as an array only if needed, or remove it entirely before merge. - In
scripts/scr_random_event/scr_random_event.gml, duplicated weighting logic for events likeFLEET_DELAYandSHIP_LOSTincreases the chance of behavior drift if one branch is updated and the other is missed—extract the shared “any moving fleet” check into a helper to de-risk future edits. - In
objects/obj_bomb_select/Draw_0.gml, dead locals (_total_hers,_influ,str_string) add noise after refactoring and can mislead maintainers about active draw logic—clean them up in this PR or as an immediate follow-up.
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:37">
P1: Removed `_heading = headings[i];` after string-to-ReactiveString conversion leaves `_heading` pointing to the original string, causing invalid `.w` and `.update()` access.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
2b07c6e to
bd42693
Compare
There was a problem hiding this comment.
2 issues found across 45 files
Confidence score: 2/5
- In
scripts/scr_crusade/scr_crusade.gml, overlappingelse ifbranches make thebrutalcrusade path unreachable, so that game mode effectively cannot be triggered after merge. Reorder or split the conditions sobrutalhas a reachable branch (and add a quick coverage check for each crusade type) before merging. - In
scripts/scr_image/scr_image.gml, the JSDoc still documents-1while the code now returnsundefined, and it lacks an explicit@returns, which can mislead maintainers and integrations that rely on docs/tooling. Update the JSDoc to match current behavior before merging to reduce follow-on bugs.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
bd42693 to
26845b7
Compare
There was a problem hiding this comment.
1 issue found across 45 files
Confidence score: 2/5
- In
scripts/scr_event_newlines/scr_event_newlines.gml, the fill loop is skipping the first open line index (lo), which can write incorrect'---'values and corrupt newline/event formatting behavior for users; this is a concrete logic bug, so fix the loop bounds/index handling and add a focused test for theloedge case before merging.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
26845b7 to
9f9ce1a
Compare
There was a problem hiding this comment.
1 issue found across 45 files
Confidence score: 4/5
- In
objects/obj_event_log/Mouse_60.gml(and relatedobj_event_loghandlers), hardcoded scroll-step value2can drift across files and cause inconsistent scroll behavior if one copy is changed later; this is a maintainability risk more than an immediate bug, so this PR is likely safe to merge, but defining a shared#macrobefore or immediately after merge will de-risk future regressions.
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="objects/obj_event_log/Mouse_60.gml">
<violation number="1" location="objects/obj_event_log/Mouse_60.gml:2">
P2: Custom agent: **Code Quality Review**
Raw number `2` used as a scroll-step constant in multiple files of `obj_event_log`; should be a #macro.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Ready to merge |
There was a problem hiding this comment.
1 issue found across 45 files
Confidence score: 5/5
- This PR looks safe to merge; the main risk is maintainability in
scripts/scr_gift_items/scr_gift_items.gml, where the repeated magic number131can drift if one occurrence is updated and the other is missed, potentially causing inconsistent item positioning later—extract it into a shared#macro/enum constant when convenient (or before merge if you want to prevent follow-up cleanup).
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="objects/obj_event_log/Mouse_60.gml">
<violation number="1" location="objects/obj_event_log/Mouse_60.gml:2">
P2: Custom agent: **Code Quality Review**
Raw number `2` used as a scroll-step constant in multiple files of `obj_event_log`; should be a #macro.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
i stand by that statement bot |
The bot will specify everything, but yeah just a lot of variables that don't need to be there removed, others have their definitions tuned up to respect typing, or be in-lined, and loop tightening.
Summary by cubic
Refactor to tighten variable scope, replace repeat loops, and standardize sentinels to noone across ships, stars, and helpers. UI drawing is cleaner (Event Log, stat panel), with improved scr_image loading/drawing and smoother Event Log scrolling.
Refactors
Bug Fixes
Written for commit 9f9ce1a. Summary will update on new commits.