Skip to content

refactor: More stray variable definition and loop tune ups.#1250

Draft
carys-the-weed-cloud wants to merge 1 commit into
Adeptus-Dominus:mainfrom
carys-the-weed-cloud:main
Draft

refactor: More stray variable definition and loop tune ups.#1250
carys-the-weed-cloud wants to merge 1 commit into
Adeptus-Dominus:mainfrom
carys-the-weed-cloud:main

Conversation

@carys-the-weed-cloud

@carys-the-weed-cloud carys-the-weed-cloud commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

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

    • Replaced more repeat loops with for; pre-sized arrays for help topics and dropdowns; streamlined save listing and INI/topic reads.
    • Standardized sentinels/targets (noone) in ship targeting and map helpers; simplified turret/boarding code and movement tuning.
    • Map/render: cleaner warp lane building and star-box handling; scr_image cache/cleanup uses bounded loops and clear fallbacks; consistent draw colors.
    • UI: consolidated Event Log/help topic rendering; unit stat panel uses correct camera origin and unit name lookup.
  • Bug Fixes

    • Crusade difficulty range corrected (51–80% is “hard”); fixed shadowed/undefined vars in scr_load (JSON scope), scr_Table (heading), scr_move_unit_info, and TradeAttempt (request count).
    • Event Log: scrolls two lines per wheel step; unified scrollbar math and removed duplicate handlers.
    • Prevented OOB/layout issues (help topic arrays, gift UI stacking, dropdown target lock) and fixed whole-squad transfer indexing.

Written for commit 9f9ce1a. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot added Size: Huge Type: Refactor Rewriting/restructuring code, while keeping general behavior labels Jun 18, 2026
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as ready for review June 18, 2026 00:27

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 45 files

Confidence score: 3/5

  • In scripts/scr_Table/scr_Table.gml, removing _heading = headings[i]; after converting strings to ReactiveString can leave _heading as a plain string, so later .w/.update() calls may fail at runtime and break table heading rendering—restore the reassignment (or otherwise guarantee _heading is the reactive object) before merging.
  • In scripts/scr_crusade/scr_crusade.gml, clean is initialized as scalar 0 and 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 like FLEET_DELAY and SHIP_LOST increases 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

Comment thread scripts/scr_Table/scr_Table.gml
Comment thread scripts/scr_random_event/scr_random_event.gml
Comment thread scripts/scr_crusade/scr_crusade.gml Outdated
Comment thread objects/obj_bomb_select/Draw_0.gml Outdated
Comment thread scripts/scr_crusade/scr_crusade.gml Outdated
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as draft June 18, 2026 00:39
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as ready for review June 18, 2026 00:39

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 45 files

Confidence score: 2/5

  • In scripts/scr_crusade/scr_crusade.gml, overlapping else if branches make the brutal crusade path unreachable, so that game mode effectively cannot be triggered after merge. Reorder or split the conditions so brutal has 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 -1 while the code now returns undefined, 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

Comment thread scripts/scr_crusade/scr_crusade.gml
Comment thread scripts/scr_image/scr_image.gml
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as draft June 18, 2026 00:55
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as ready for review June 18, 2026 00:55

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the lo edge case before merging.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/scr_event_newlines/scr_event_newlines.gml Outdated
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as draft June 18, 2026 01:02
@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as ready for review June 18, 2026 01:05

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 45 files

Confidence score: 4/5

  • In objects/obj_event_log/Mouse_60.gml (and related obj_event_log handlers), hardcoded scroll-step value 2 can 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 #macro before 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

Comment thread objects/obj_event_log/Mouse_60.gml
@carys-the-weed-cloud

Copy link
Copy Markdown
Contributor Author

Ready to merge

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 number 131 can 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

Comment thread scripts/scr_gift_items/scr_gift_items.gml
@carys-the-weed-cloud

Copy link
Copy Markdown
Contributor Author

i stand by that statement bot

@carys-the-weed-cloud carys-the-weed-cloud marked this pull request as draft June 18, 2026 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: Huge Type: Refactor Rewriting/restructuring code, while keeping general behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant