feat: Lightning Warriors with a small extra - #1300
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 33 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| var _src = struct_exists(_arrangement, "default_squads") ? _arrangement.default_squads : []; | ||
| var _squads = array_create(array_length(_src)); | ||
| for (var _i = 0; _i < array_length(_src); _i++) { | ||
| _squads[_i] = variable_clone(_src[_i]); | ||
| } |
There was a problem hiding this comment.
same ass above variable_clone natrually clones depth so no need for loops
| @@ -194,5 +184,79 @@ function role_hierarchy() { | |||
| "Ork Sniper" | |||
| ]; | |||
|
|
|||
There was a problem hiding this comment.
this whole logic just repeats a logic already covered in the company sort but adds a lot of complexity, to fix sgts displaying above other squad members just ensure the sgt is placed in front of other roles inn the company sort otherwise this dynamic logic needs too be repeated everytime the hierarchy us required for any reason
There was a problem hiding this comment.
This isn't just a display fix, determine_leader and hierarchy_keys also eat this, so this is a critical bit of code for all renamed sergeant roles. I will make it less shit, but just pulling the load of this to the defaults will break all custom sergeant types
| if (scr_has_adv("Lightning Warriors")) { | ||
| obj_ini.chapter_squad_arrangement = json_to_gamemaker( | ||
| working_directory + $"main\\squads\\lightning_warriors.json", json_parse); | ||
| var _dist_key = ""; | ||
| switch (obj_creation.squad_distribution) { | ||
| case 1: _dist_key = "equal_specialists"; break; | ||
| case 2: _dist_key = "equal_scouts"; break; | ||
| case 3: _dist_key = "equal_spescout"; break; | ||
| } | ||
| if (_dist_key != "" | ||
| && struct_exists(obj_ini.chapter_squad_arrangement, "distribution_overrides") | ||
| && struct_exists(obj_ini.chapter_squad_arrangement.distribution_overrides, _dist_key)) { | ||
| apply_squad_distribution_override( | ||
| obj_ini.chapter_squad_arrangement, | ||
| obj_ini.chapter_squad_arrangement.distribution_overrides[$ _dist_key]); | ||
| } | ||
| } else { | ||
| switch (obj_creation.squad_distribution) { | ||
| case 1: // equal specialists only | ||
| obj_ini.chapter_squad_arrangement = json_to_gamemaker( | ||
| working_directory + $"main\\squads\\equal_specialists.json", json_parse); | ||
| break; | ||
| case 2: // equal scouts only | ||
| obj_ini.chapter_squad_arrangement = json_to_gamemaker( | ||
| working_directory + $"main\\squads\\equal_scouts.json", json_parse); | ||
| break; | ||
| case 3: // equal specialists and equal scouts | ||
| obj_ini.chapter_squad_arrangement = json_to_gamemaker( | ||
| working_directory + $"main\\squads\\equal_spescout.json", json_parse); | ||
| break; | ||
| default: // 0 = standard | ||
| obj_ini.chapter_squad_arrangement = json_to_gamemaker( | ||
| working_directory + $"main\\squads\\company_squad_builds.json", json_parse); | ||
| break; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Double slashes are obsolete now, broken on Linux. We use / for paths now.
| LIBRARIAN = 17, | ||
| SERGEANT = 18, | ||
| VETERANSERGEANT = 19, | ||
| ATTACK_BIKER = 20, |
There was a problem hiding this comment.
I'm not sure if ATTACK_BIKER role is needed. Following the established convention, we don't separate roles like that. Assault Terminator is just a Terminator. I'm also not sure as to benefits of this separation in this exact case.
There was a problem hiding this comment.
distinct promotion entries, squad-type fetch pools and roster display is what the benefits are.
To expand:
- I can easily expand on their different combat role in the future PRs and add missions that are antirely AB related cause they get a separate erole
- clarity of display, chapter management would only display X Bikers if the AB were not a separate erole entry since both this and the welcome message just builds off strings
- they have their own squad sort and promotion pipeline so you can actually create an AB squad in the current promotion screen, otherwise it'd be impossible to create any AB squad midgame
- on squad teardowns if he's tracked as an ex-ABiker he won't get folded into Biker squads first, so less reequipment in the background
There is an argument that the combat logic doesn't need them, I think I can get away with making the fetch pools and the roster display work without the erole, but the promotion entry [which is a core feature for these base squads imo] would get killed, as well as their default gear entry
There was a problem hiding this comment.
I wanted to say something here, like:
- When and if you expand on it, you can add a separate role. Or easier - make it depend on the squad, not the role.
- Clarity of display is not a super big issue either. The only difference between Bikers and AB is that they use different bike items, right? Like... okay.
But the arguments about not being able to create new AB squads or quickly reequip via promote - are sound.
We already have the same issue with assault and normal termies, and I hate it.
Same was with sternguard and vanguard (which led me to merging both).
So for as long as squads are not super flexible and don't interact with promotions well - I don't care. I wouldn't care even if people add Assault Terminator role at this point.
If and when we'll have proper working squad customization, promotion control - then we can delete the roles, it won't take too long. Otherwise, it's just having semi-working roles and coping that it'll change soon.
...BUT! As I was writing this, I actually came to a conclusion that neither AB role, nor squad are needed. I think you can simply wire attack bikes as a random loadout for normal biker squads. And I think that makes more sense.
| var _is_vsgt = (_k == _vsgt_base) || (string_count(_vsgt_base, _specific_role) > 0); | ||
| var _is_sgt = (_k == _sgt_base) || (string_count(_sgt_base, _specific_role) > 0); |
There was a problem hiding this comment.
Not 100% sure if I'm reading this correctly, but string_count() takes "what" to search for as the first argument, and "where" to search as the second. It seems the intent here is reversed, isn't it?
Shouldn't it be:
var _is_vsgt = (_k == _vsgt_base) || (string_count(_specific_role, _vsgt_base) > 0);
var _is_sgt = (_k == _sgt_base) || (string_count(_specific_role, _sgt_base) > 0);There was a problem hiding this comment.
actually no, this bit checks whether a _specific_role (so the renamed to flavour role string on leader slot like "Biker Sergeant", which update_role writes as a string in his role, so role_hierarchy() would just have the role fall to the bottom of the sort and not be matched as Sergeant-tier) contains a sgt or vsgt base, without this a non-basic "Sergeant" would no longer be highest in the troop list from his squad, just wherever really.
so this is intended behaviour
There was a problem hiding this comment.
Though I will give this a second look cause Nelson pointed to a weak link somewhere else that would change this
| // candidate or marking sergeant_found. Otherwise a sergeant whose role the squad has | ||
| // no slot for would be added anyway and incorrectly suppress the promotion flow. | ||
| var _sgt_group = ""; | ||
| for (var r = 0; r < array_length(squad_unit_types); r++) { |
There was a problem hiding this comment.
The pattern of:
- iterating
squad_unit_types - reading
_fill_squad[$ _role_name] - extracting
_primary_role_name = struct_exists(_role_def, "role") ? _role_def.role : _role_name - and matching against
sgt_types
Is copy-pasted in the new code 5(!!!!) times.
This exact sergeant block is copy-pasted lower, in _has_sgt_requirements check and then again in promotion _sgt_group resolution.
There was a problem hiding this comment.
that's on me being lazy, I'll collapse it
| if (_target_sgt_role != _sgt.role()) { | ||
| _sgt.update_role(_target_sgt_role); | ||
| } |
There was a problem hiding this comment.
I think this renames a pre-existing sergeant before the squad viability check later down. If the squad creation fails, the sergeant has been permanently renamed for nothing. I thought logic outlined in deferred-promotion wanted to avoid such cases.
There was a problem hiding this comment.
damn you right. Probably an oversight when I was changing approaches, will amend
| var _squad_type_names = struct_get_names(obj_ini.squad_types); | ||
| for (var _si = 0; _si < array_length(_squad_type_names); _si++) { | ||
| var _sq_data = obj_ini.squad_types[$ _squad_type_names[_si]]; | ||
| var _sq_keys = struct_get_names(_sq_data); | ||
| for (var _ki = 0; _ki < array_length(_sq_keys); _ki++) { | ||
| var _k = _sq_keys[_ki]; | ||
| if (_k == "type_data") continue; | ||
| var _role_def = _sq_data[$ _k]; | ||
| if (!struct_exists(_role_def, "role")) continue; | ||
| var _specific_role = _role_def.role; | ||
| if (!array_contains(hierarchy, _specific_role)) { | ||
| // Classify by the slot's JSON key (_k), not the renamed role string. Veteran-sergeant | ||
| // variants are keyed "Veteran Sergeant" but get renamed to names like "Deathwing | ||
| // Sergeant" / "Proteus Watch Sergeant" that contain "Sergeant" but NOT the exact | ||
| // substring "Veteran Sergeant" — so a role-string match would mis-rank them as | ||
| // regular sergeants. Fall back to string matching only when the key isn't a sergeant. | ||
| var _is_vsgt = (_k == _vsgt_base) || (string_count(_vsgt_base, _specific_role) > 0); | ||
| var _is_sgt = (_k == _sgt_base) || (string_count(_sgt_base, _specific_role) > 0); | ||
| if (_is_vsgt) { | ||
| // Veteran-sergeant variant — insert just before _vsgt_base position | ||
| var _vpos = array_get_index(hierarchy, _vsgt_base); | ||
| array_insert(hierarchy, max(0, _vpos), _specific_role); | ||
| } else if (_is_sgt) { | ||
| // Regular sergeant variant — insert just after _sgt_base position | ||
| var _spos = array_get_index(hierarchy, _sgt_base); | ||
| array_insert(hierarchy, _spos + 1, _specific_role); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Also add non-sergeant squad-specific role variants so they appear after their base role | ||
| var _base_roles = [ | ||
| _roles[eROLE.TERMINATOR], _roles[eROLE.VETERAN], | ||
| _roles[eROLE.TACTICAL], _roles[eROLE.ASSAULT], | ||
| _roles[eROLE.DEVASTATOR], _roles[eROLE.SCOUT], | ||
| _roles[eROLE.ANCIENT], _roles[eROLE.CHAMPION], | ||
| _roles[eROLE.CHAPLAIN], _roles[eROLE.APOTHECARY], | ||
| _roles[eROLE.TECHMARINE], _roles[eROLE.LIBRARIAN] | ||
| ]; | ||
| for (var _si = 0; _si < array_length(_squad_type_names); _si++) { | ||
| var _sq_data = obj_ini.squad_types[$ _squad_type_names[_si]]; | ||
| var _sq_keys = struct_get_names(_sq_data); | ||
| for (var _ki = 0; _ki < array_length(_sq_keys); _ki++) { | ||
| var _k = _sq_keys[_ki]; | ||
| if (_k == "type_data") continue; | ||
| var _role_def = _sq_data[$ _k]; | ||
| if (!struct_exists(_role_def, "role")) continue; | ||
| var _specific_role = _role_def.role; | ||
| if (array_contains(hierarchy, _specific_role)) continue; | ||
| // Skip sergeant variants (already handled above) | ||
| if (string_count(_sgt_base, _specific_role) > 0) continue; | ||
| // Find the closest matching base role and insert after it | ||
| for (var _bi = 0; _bi < array_length(_base_roles); _bi++) { | ||
| if (struct_exists(_role_def, "alternative_roles") && | ||
| array_contains(_role_def.alternative_roles, _base_roles[_bi])) { | ||
| var _bpos = array_get_index(hierarchy, _base_roles[_bi]); | ||
| if (_bpos >= 0) { | ||
| array_insert(hierarchy, _bpos + 1, _specific_role); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| // If not inserted via alternative_roles, just append before rank-and-file | ||
| if (!array_contains(hierarchy, _specific_role)) { | ||
| array_push(hierarchy, _specific_role); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not 100% sure, but I think the addition of this code block started to freeze the game on each opening of chapter view, because sort_all_companies()>scr_company_order() are called every time the window is opened.
I'm not sure if it's this exact path that is bad, but I suspect it is.
No caching. The hierarchy is rebuilt from scratch every call.
There was a problem hiding this comment.
It is, I'll give it a once-over
|
And of course: ugly AI slop with 99999999 lines of verbose comments I don't need. Aside from everything stated, some of the changes, new functionality and fixes are cool. |
ca1ab91 to
d62b258
Compare
|
|
||
| } else if (_unit_role == _role[13]) { | ||
| // bikers | ||
| col = obj_controller.bat_bike_column; |
There was a problem hiding this comment.
CRITICAL: Missing new_combat.bikers++ for biker role branch
Every other unit branch in this if/else if chain increments a new_combat.* counter (e.g. new_combat.scouts++, new_combat.assaults++, new_combat.librarians++). The biker branch at line 575 and the attack biker branch at line 578 set col but never increment any counter, producing incorrect battle statistics and potentially breaking downstream logic that depends on these counts.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| col = obj_controller.bat_bike_column; | ||
| } else if (_unit_role == _role[20]) { | ||
| //assbikers | ||
| col = obj_controller.bat_bike_column; |
There was a problem hiding this comment.
CRITICAL: Missing new_combat.attack_bikers++ for attack biker role branch
Every other unit branch in this if/else if chain increments a new_combat.* counter (e.g. new_combat.scouts++, new_combat.assaults++). The attack biker branch at line 578 sets col but never increments any counter, producing incorrect battle statistics and potentially breaking downstream logic that depends on these counts.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "role": "Librarian", | ||
| "loadout": { | ||
| "required": { | ||
| "wep2": ["", 0], |
There was a problem hiding this comment.
CRITICAL: Librarian wep2 slot permanently cleared on squad equipment sort
Adding "wep2": ["", 0] to the Librarian's required loadout makes wep2 a managed slot with a target count of 0. The equipment sort function clears all managed slots to "" before re-equipping, but since the required target is 0, the slot is never re-filled. The Librarian's default Bolt Pistol (set up in load_default_gear) is permanently lost every time formations are sorted.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "required": { | ||
| "wep1": ["Relic Blade", 1], | ||
| "wep2": ["Plasma Pistol", 1], | ||
| "wep2": ["", 0], |
There was a problem hiding this comment.
CRITICAL: Huntsmaster wep2 sidearm permanently cleared on squad equipment sort
Changing "wep2": ["Plasma Pistol", 1] to "wep2": ["", 0] makes wep2 a managed slot with a target count of 0. The equipment sort function clears all managed slots to "" before re-equipping, but since the required target is 0, the slot is never re-filled. The Huntsmaster's Plasma Pistol is permanently lost every time formations are sorted.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| break; | ||
| if (scr_has_adv("Lightning Warriors")) { | ||
| obj_ini.chapter_squad_arrangement = json_to_gamemaker( | ||
| working_directory + $"main\\squads\\lightning_warriors.json", json_parse); |
There was a problem hiding this comment.
WARNING: Backslash path separators \\ in new paths contradict established codebase convention
All five newly added squad JSON file paths (lines 1654, 1672, 1676, 1680, 1684) use main\\squads\\... with escaped backslashes. An existing comment on this codebase (line 1688) explicitly states: "Double slashes are obsolete now, broken on Linux. We use / for paths now." All pre-existing squad file paths in this function use forward slashes. These five new paths break consistency and will fail on Linux/macOS where backslash is a literal character, not a path separator.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Previous Review Summaries (2 snapshots, latest commit 8d7e6fe)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8d7e6fe)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Previous review (commit d62b258)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (3 files)
Reviewed by step-3.7-flash · Input: 313.8K · Output: 41K · Cached: 2.1M |
d62b258 to
8d7e6fe
Compare
8d7e6fe to
1800b20
Compare
| col = obj_controller.bat_assault_column; | ||
| new_combat.assaults++; | ||
|
|
||
| } else if (_unit_role == _role[13]) { |
There was a problem hiding this comment.
[CRITICAL]: Bikers and Attack Bikers missing new_combat battle counters
The add_unit_to_battle function increments new_combat.* counters for every role branch (tacticals, veterans, devastators, assaults, etc.) but the new biker (_role[13]) and attack biker (_role[20]) branches do not increment new_combat.bikers++ or new_combat.attack_bikers++. Battle summary statistics for bikes will always be zero, and any downstream logic reading those counters will behave incorrectly.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| break; | ||
| case "biker": | ||
| col = obj_controller.bat_bike_column; | ||
| break; |
There was a problem hiding this comment.
[CRITICAL]: Missing "attack_biker" case in formation_place switch
The switch at line 689 handles "biker" but not "attack_biker". Attack bike squads whose formation_place is "attack_biker" fall through the switch, leaving col == 0 which defaults to the hireling column. Attack bikes will be placed in the wrong battle block.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| squad_fulfilment[$ _wanted_unit_role] = 0; //create a fulfilment structure to log members of squad | ||
| var _role_def = fill_squad[$ _wanted_unit_role]; | ||
| var _mapped = struct_exists(_role_def, "role") ? _role_def.role : _wanted_unit_role; |
There was a problem hiding this comment.
[WARNING]: update_fulfillment _actual_to_key mapping omits alternative_roles
The _actual_to_key map (lines 464-474) only covers slots with a primary "role" override. If a squad slot defines "alternative_roles": ["Attack Biker"] but no slot has "role": "Attack Biker", marines carrying the alternative role fall back to their literal role name. squad_fulfilment["bike_squad_key"] stays at 0 while squad_fulfilment["Attack Biker"] is incremented, causing the min/max fulfilment check to falsely report the squad as underfilled. This can cause valid squads to be emptied during scr_company_order.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (_ranged) { | ||
| // Attack Bike: scales off the "sidecar's" ranged weapon - single firepower profile, no melee option. | ||
| var _attack = ranged_damage_data[0]; | ||
| var _weapon = ranged_damage_data[3]; |
There was a problem hiding this comment.
[WARNING]: speed_force assumes ranged_damage_data[3] is a struct with spli/arp
The new speed_force function (line 1668) accesses ranged_damage_data[3].spli and ranged_damage_data[3].arp without checking that index 3 exists or holds a struct. Attack bikes can be spawned with loadouts that leave ranged_damage_data uninitialized or with non-struct entries at index 3. A nil/type-mismatch access here will crash at runtime when a bike attacks.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| // Standard Bike: scales off melee, dominant melee (M) profile while engaged in front. | ||
| var _melee_attack = melee_damage_data[0]; | ||
| var _melee_weapon = melee_damage_data[3]; |
There was a problem hiding this comment.
[WARNING]: speed_force assumes melee_damage_data[3] is a struct with spli/arp
The new speed_force function (line 1668) accesses melee_damage_data[3].spli and melee_damage_data[3].arp without checking that index 3 exists or holds a struct. Standard bikes can be spawned with loadouts that leave melee_damage_data uninitialized or with non-struct entries at index 3. A nil/type-mismatch access here will crash at runtime when a bike attacks.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| } | ||
| if (!_found) { | ||
| array_push(arrangement.companies, _ovr); |
There was a problem hiding this comment.
[WARNING]: apply_squad_distribution_override inserts company entries by reference
The old code deep-cloned each company entry before inserting it into arrangement.companies. The new code assigns arrangement.companies[ai] = _ovr and array_push(arrangement.companies, _ovr) directly, sharing the override's struct reference. Any future in-place mutation of the override (or reusing the same override struct across chapters) will corrupt the live chapter_squad_arrangement.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| for (var d = 0; d < array_length(_veh_names); d++) { | ||
| if (_veh_count[d] == 1) { | ||
| array_push(pane.line, {str1: _veh_names[d], bold: true, italic: false}); |
There was a problem hiding this comment.
[WARNING]: Predator vehicle display no longer always bold
The old code bolded the Predator entry (index 1) whenever its count was greater than zero. The new code only bolds when the count equals exactly one. This silently changes the management screen UI for Predators: multi-Predator companies now display without the bold highlight that previously distinguished them.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
This PR is marked as stale, because it has been open for 7 days with no activity. |
Summary
New squad types, formation bit, template redone, company composition changes, cleaning repetitions in formation bar drag and in the welcome message
Refactors some combat-relevant stuff into more accurate for bikers, adds placeholder Speed Force
Accounts for new mobility hands with small squad setup redones
ID for flesh tearers moved for visibility
Testing
shit works
Summary by cubic
Adds the Lightning Warriors preset and full Biker/Attack Biker support across roles, formations, squad templates, and UI. Also rebalances Bike mobility, updates pistol/bike-ready loadouts, improves auto-setup and equipment sorting, and fixes formation persistence, armour setup, promotions, and management displays.
New Features
formation18.pngfor Biker formations; pistol‑first sidearm updates in chapter entries.BIKERandATTACK_BIKERroles with default gear, promotion targets (bike_squad,attack_bike_squad), UI/settings, and a Biker formation bar/column.biketags.alternative_rolesand multi‑role fills; runtime company layout resolver; equipment sorting respects slot groups.Bug Fixes
idplacement corrected.Written for commit 1800b20. Summary will update on new commits.