refactor: Fixing some misc bugs.#1307
Open
carys-the-weed-cloud wants to merge 6 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
6 issues found across 9 files
Confidence score: 2/5
- In
objects/obj_pnunit/Alarm_0.gml, the infantry and medic targeting fallthrough paths can readenemy.men/enemy.mediafter a prior shot destroysenemy, creating a concrete runtime error risk during combat resolution—restoreinstance_exists(enemy)guards (or equivalent early exit) before those reads before merging. - In
objects/obj_pnunit/Alarm_0.gml, the newexitpath can skip the laterinstance_activate_object(obj_enunit)cleanup, leaving flanked enemy units deactivated for the rest of combat—route this branch through shared cleanup or reactivateobj_enunitbefore exiting. - In
scripts/__init/__init.gml, duplicatesubcomponentskeys in Cataphractii struct literals silently overwrite earlier values, which can mask intended equipment composition and make behavior/config harder to trust—deduplicate each struct key so only the intended array is defined once before merge.
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_pnunit/Alarm_0.gml">
<violation number="1" location="objects/obj_pnunit/Alarm_0.gml:148">
P1: A ranged AP shot that kills the first target can now fall through into the medic targeting block and read `enemy.medi` on a destroyed instance. Keeping the `instance_exists(enemy)` guard here preserves the intended post-shot safety check that this refactor removed.</violation>
<violation number="2" location="objects/obj_pnunit/Alarm_0.gml:175">
P1: A shot in the earlier ranged targeting blocks can destroy `enemy` and still let execution reach infantry targeting, where `enemy.men` is read without the removed existence check. Re-adding `instance_exists(enemy)` to this condition would prevent intermittent combat exceptions after a target dies mid-weapon.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
307559b to
9dcc698
Compare
9dcc698 to
a65e7c1
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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_pnunit/Alarm_0.gml">
<violation number="1" location="objects/obj_pnunit/Alarm_0.gml:146">
P2: When a volley destroys the final enemy inside one of these branches, remaining ranged weapons now skip the existing `report_held_fire` path and the battle log omits their “held fire” message. Consider reporting held fire before these new `break`s, or setting a flag so the loop-level empty-target handler still runs.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
4 issues found across 11 files
Confidence score: 3/5
- In
objects/obj_pnunit/Alarm_0.gml, the newbreakcan exit the weapon loop as soon as one targetedenemydies, so remaining enemies may stop receiving fire for that tick and combat behavior can regress — only skip/reacquire when the current target is invalid instead of breaking the full weapon iteration. - In
objects/obj_al_ship/Step_0.gml, the new early-exit path can bypass theowner == eFACTION.ELDARhp <= 0cleanup when the last enemy is gone, leaving destroyed allied Eldar ships unprocessed — ensure death/cleanup logic still runs even when no enemies remain. - Also in
objects/obj_al_ship/Step_0.gml, duplicated targeting/firing logic across Eldar and non-Eldar branches raises drift risk, where future fixes land in one branch but not the other — extract the shared loop into one code path before or immediately after merge to reduce regression risk. - In
scripts/__init/__init.gml, the “12 is skipped” comment is inaccurate and could mislead future maintenance, though runtime risk is low — update the comment to match the actual index assignment.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
48dcdf1 to
93520a7
Compare
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.
The first commit was more like a chore/fix but still felt more appropriate to call it a refactor. A little cleaning of combat code, and fixing a bug where by your unit tries to fight an empty battlefield. Bot, analysis.
Summary by cubic
Fixes units and ships acting without a valid target, centralizes force/star globals in
__init(), and streamlines allied ship combat with clearer targeting and shield logic.Bug Fixes
obj_pnunit/Alarm_0: Disengage whenenemyis missing; add early-exit checks after each veh/medi/men block (ranged/melee) to stop acting on destroyed enemies.unit = undefinedinobj_pnunit/obj_enunit, setcm_block = nooneinobj_enunit/Alarm_0, and clean upscr_culture_visualsoverlays to avoid invalid states.Refactors
force_strength_descriptionsandstar_name_colorstoscripts/__init/__init.gml, removing duplicates fromscr_PlanetDataandobj_star.obj_al_ship: run only whenobj_fleet.start == 5, target nearestobj_en_shipand exit if none, syncimage_angletodirection, defaultowner = eFACTION.IMPERIUM, useeFACTION/nooneconsistently, and keep faster shield regen foreFACTION.ELDAR.Written for commit 93520a7. Summary will update on new commits.