refactor(perf): add_marines_to_recovery move switch to dict lookup

[SweetZJ]

I refactored the add_marines_to_recovery function because before that logic was a giant nest of if statements and I was taught that is not great practise so I attempted to simplify it by:

• creating a structure to hold all of the values which says how important a marine is to apothacary instead of the switch case used inside of the for loop which goes through every killed marine

• reducing the amount of nested if statements by using continue after the conditional, just to make it look better

  Testing:
  I'm new and have basically no clue how to test, so I just ran the program, fought ten battles before the change, and then fought ten battles against similar opposing forces after, and what died and what got revived SEEMED to be somewhat similar.

---

Summary by cubic

Refactors add_marines_to_recovery to replace the nested switch/if logic with a lookup struct and early continue guards without changing behavior.

• Role priority bonuses are now stored in a _bonus_specific struct keyed by role title; roles not listed (e.g. Scout) fall back to 0.
• The flattened control flow keeps the same priority calculation and recovery candidate creation as before.

^{Written for commit 7a4dff4. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 1 file

Confidence score: 3/5

• In scripts/scr_after_combat/scr_after_combat.gml, the struct lookup may change duplicate-title precedence: entries that previously selected the first, higher-bonus switch case could now resolve differently, affecting post-combat bonuses — preserve the original top-down precedence or add coverage for duplicate titles.
• The one-key-per-assignment table in scripts/scr_after_combat/scr_after_combat.gml is less maintainable than the nearby add_vehicles_to_recovery struct-literal pattern, increasing the chance of future table-edit mistakes — align the representation with the established pattern.

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_after_combat/scr_after_combat.gml">

<violation number="1" location="scripts/scr_after_combat/scr_after_combat.gml:6">
P2: The struct lookup changes precedence semantics from the old switch without flagging it. The switch matched the first case top-down, so when two table entries shared a title the earlier (higher-bonus) case won; in the new table the last assignment wins, so the later (lower-bonus) row silently overwrites it. Example: if a chapter renames the Apothecary to "Forge Master", the old code hit `case "Forge Master"` first and gave 360, while the new code's later `_bonus_specific[$ _roles[eROLE.APOTHECARY]] = 40` overwrites it with 40. Role titles are config data, not constants: `active_roles()` returns `obj_creation.role[100]`/`obj_ini.role[100]` (scripts/is_specialist/is_specialist.gml) and `_unit.role()` returns the stored title string `obj_ini.role[company][marine_number]` (scripts/scr_marine_struct/scr_marine_struct.gml), so two roles can legitimately hold the same title. A dropped bonus changes who `ds_priority_delete_max` revives first in obj_ncombat/Alarm_5.gml. Build the table from an ordered [title, bonus] list with first-write-wins, or define priorities as `#macro`s and construct a single literal so collisions are visible instead of order-dependent.</violation>

<violation number="2" location="scripts/scr_after_combat/scr_after_combat.gml:12">
P3: This 24-line one-key-per-assignment table is harder to maintain than the established pattern: the function directly below, `add_vehicles_to_recovery`, uses a single struct literal (`{ "Land Raider": 10, ... }`) for the same lookup. A literal is shorter, matches the neighbouring code, and makes a duplicated key visible at compile time instead of silently depending on assignment order. The stray blank lines between priority tiers read as leftover scaffolding; a tier comment or grouped literal conveys the intent instead. While rewriting, also consider the repo style (`.coderabbit.yaml`) of `#macro`/`enum` over bare literals: 720/360/160/80/40/20 and the hardcoded titles like "Forge Master", "Codiciery", and "Lexicanum" are raw values.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The struct lookup changes precedence semantics from the old switch without flagging it. The switch matched the first case top-down, so when two table entries shared a title the earlier (higher-bonus) case won; in the new table the last assignment wins, so the later (lower-bonus) row silently overwrites it. Example: if a chapter renames the Apothecary to "Forge Master", the old code hit case "Forge Master" first and gave 360, while the new code's later _bonus_specific[$ _roles[eROLE.APOTHECARY]] = 40 overwrites it with 40. Role titles are config data, not constants: active_roles() returns obj_creation.role[100]/obj_ini.role[100] (scripts/is_specialist/is_specialist.gml) and _unit.role() returns the stored title string obj_ini.role[company][marine_number] (scripts/scr_marine_struct/scr_marine_struct.gml), so two roles can legitimately hold the same title. A dropped bonus changes who ds_priority_delete_max revives first in obj_ncombat/Alarm_5.gml. Build the table from an ordered [title, bonus] list with first-write-wins, or define priorities as #macros and construct a single literal so collisions are visible instead of order-dependent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_after_combat/scr_after_combat.gml, line 6:

<comment>The struct lookup changes precedence semantics from the old switch without flagging it. The switch matched the first case top-down, so when two table entries shared a title the earlier (higher-bonus) case won; in the new table the last assignment wins, so the later (lower-bonus) row silently overwrites it. Example: if a chapter renames the Apothecary to "Forge Master", the old code hit `case "Forge Master"` first and gave 360, while the new code's later `_bonus_specific[$ _roles[eROLE.APOTHECARY]] = 40` overwrites it with 40. Role titles are config data, not constants: `active_roles()` returns `obj_creation.role[100]`/`obj_ini.role[100]` (scripts/is_specialist/is_specialist.gml) and `_unit.role()` returns the stored title string `obj_ini.role[company][marine_number]` (scripts/scr_marine_struct/scr_marine_struct.gml), so two roles can legitimately hold the same title. A dropped bonus changes who `ds_priority_delete_max` revives first in obj_ncombat/Alarm_5.gml. Build the table from an ordered [title, bonus] list with first-write-wins, or define priorities as `#macro`s and construct a single literal so collisions are visible instead of order-dependent.</comment>

<file context>
@@ -1,67 +1,70 @@
 function add_marines_to_recovery() {
     var _roles = active_roles();
+    
+    var _bonus_specific = {};
+    
+
</file context>

[cubic-dev-ai]

P3: This 24-line one-key-per-assignment table is harder to maintain than the established pattern: the function directly below, add_vehicles_to_recovery, uses a single struct literal ({ "Land Raider": 10, ... }) for the same lookup. A literal is shorter, matches the neighbouring code, and makes a duplicated key visible at compile time instead of silently depending on assignment order. The stray blank lines between priority tiers read as leftover scaffolding; a tier comment or grouped literal conveys the intent instead. While rewriting, also consider the repo style (.coderabbit.yaml) of #macro/enum over bare literals: 720/360/160/80/40/20 and the hardcoded titles like "Forge Master", "Codiciery", and "Lexicanum" are raw values.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_after_combat/scr_after_combat.gml, line 12:

<comment>This 24-line one-key-per-assignment table is harder to maintain than the established pattern: the function directly below, `add_vehicles_to_recovery`, uses a single struct literal (`{ "Land Raider": 10, ... }`) for the same lookup. A literal is shorter, matches the neighbouring code, and makes a duplicated key visible at compile time instead of silently depending on assignment order. The stray blank lines between priority tiers read as leftover scaffolding; a tier comment or grouped literal conveys the intent instead. While rewriting, also consider the repo style (`.coderabbit.yaml`) of `#macro`/`enum` over bare literals: 720/360/160/80/40/20 and the hardcoded titles like "Forge Master", "Codiciery", and "Lexicanum" are raw values.</comment>

<file context>
@@ -1,67 +1,70 @@
+    _bonus_specific[$ obj_ini.role[100][eROLE.CHAPTERMASTER]] = 720;
+    
+
+    _bonus_specific[$ "Forge Master"] = 360;
+    _bonus_specific[$ "Master of Sanctity"] = 360;
+    _bonus_specific[$ "Master of the Apothecarion"] = 360;
</file context>

[OH296]

these roles (and all marine roles) now have enums e.g eROLE.LEXICANUM

[OH296]

see lower comment regarding eums

[OH296]

obj_ini.role[100][eROLE.CHAPTERMASTER] references the struct with the roles full data the llocal var _roles = active_roles(); strips all the role strings from those structs into an array and is thus the correct ref here

Suggested change

	_bonus_specific[$ obj_ini.role[100][eROLE.CHAPTERMASTER]] = 720;
	_bonus_specific[$ _roles[eROLE.CHAPTERMASTER]] = 720;

[OH296]

in general it's easier to just create the struct and assign the values implicitly e.g
_bonus_specific = { _roles[eROLE.VETERAN]] : 40, _roles[eROLE.SERGEANT] : 40, _roles[eROLE.TECHMARINE]] : 40, etc.....

by assigning this way and decalring _bonus_specific as static not a var efficiency will likewise be improved

[OH296]

Suggested change

	if (!is_struct(_unit) || ally[i] == true) continue;
	if (marine_dead[i] != 1 || marine_type[i] == "") continue;
	if (!is_struct(_unit) || ally[i] == true){
	continue;
	}
	if (marine_dead[i] != 1 || marine_type[i] == ""){
	continue;
	}

we like to be explicit with the brackets on this project

[OH296]

Hi there, congrats on making you're first pr, a few tweaks to get this production ready also remember to solve the conflicts with main.
Any other questions just ping me here or on the discord where i am nelsonh.

[SweetZJ]

Thanks, sorry if this takes awhile I didn't really have that much free time to dedicate when I started this, but I'll get on it when I'm able.

I appreciate the help and advice tho.

[OH296]

Thanks, sorry if this takes awhile I didn't really have that much free time to dedicate when I started this

No worries do not feel hurried, as and when is fine

[SweetZJ]

@OH296 hello here, you mentioned that all roles had the eROLE thing going on but I couldn't find them for the higher-up guys such as the forge master or master apothocary, should I try to reference them like a normal veteran role or is there another way?

Also when I do attempt to do the implicit defining I am such as var _bonus_specific = {[_roles[eROLE.CHAPTERMASTER]]: 720}; or with the $ at the beginning before the _roles call I am constantly getting errors, do you know why?

Sorry to make you do what feels like all the developing but I'm still very green at all of this.

[OH296]

@OH296 hello here, you mentioned that all roles had the eROLE thing going on but I couldn't find them for the higher-up guys such as the forge master or master apothocary,

Hi, so the reason your getting this problem is because while you've been working on this branch new commits have been added to main leaving your Refactor-combat-branch behind main.

so essentially your development on the brach is developing onto of a version of the game that is 3 or 4 weeks old

To resolve this you need to merge the current development main into your Refactor-combat-branch to re-sync them.

for reference here is the major commit missing from your branch that's causing issues #1513.

as for your puposes thios repo is probably saved as the remote origin on your local system you need to merge origin.main into local. refactor-combat-branch and then resolve the ensuing conflicts

other questions are you using any sort of software to manage your git (e.g any sort of gui) or are you just using a command line?

hope this helps, again any questions just ask