feat: Marine and Vehicle recovery system adjustments #641
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughO honourable data, observe the sacred modifications: this commit replaces the experience distribution mechanism by removing the old script Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
ChapterMaster.yyp(1 hunks)objects/obj_ncombat/Alarm_5.gml(7 hunks)objects/obj_ncombat/Alarm_7.gml(2 hunks)objects/obj_ncombat/Create_0.gml(2 hunks)objects/obj_ncombat/KeyPress_13.gml(0 hunks)objects/obj_pnunit/Alarm_5.gml(0 hunks)objects/obj_pnunit/obj_pnunit.yy(1 hunks)scripts/scr_after_combat/scr_after_combat.gml(1 hunks)scripts/scr_after_combat/scr_after_combat.yy(1 hunks)scripts/scr_distribute_experience/scr_distribute_experience.gml(0 hunks)
💤 Files with no reviewable changes (3)
- scripts/scr_distribute_experience/scr_distribute_experience.gml
- objects/obj_ncombat/KeyPress_13.gml
- objects/obj_pnunit/Alarm_5.gml
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.gml`: Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ga...
**/*.gml: Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
objects/obj_ncombat/Create_0.gmlobjects/obj_ncombat/Alarm_5.gmlobjects/obj_ncombat/Alarm_7.gmlscripts/scr_after_combat/scr_after_combat.gml
`**/*.gml`: Color codes in the code shouldn't have any spaces in their id. I.e., color code `# 80 bf40` will crash the game.
**/*.gml: Color codes in the code shouldn't have any spaces in their id. I.e., color code# 80 bf40will crash the game.
objects/obj_ncombat/Create_0.gmlobjects/obj_ncombat/Alarm_5.gmlobjects/obj_ncombat/Alarm_7.gmlscripts/scr_after_combat/scr_after_combat.gml
🔇 Additional comments (25)
objects/obj_pnunit/obj_pnunit.yy (1)
39-40: Commendation: The sacred precision of integers is superior to floating-point approximations.The holy conversion from imprecise floating values (0.0, 5.0) to sacred integers (0, 5) is logically sound. This modification optimizes computational efficiency while maintaining functional equivalence, as dictated by the Omnissiah's principles of digital purity.
scripts/scr_after_combat/scr_after_combat.yy (1)
3-3: The designation has been appropriately reconsecrated to reflect its true purpose.The machine spirit of this script has been renamed from "scr_distribute_experience" to "scr_after_combat", a more encompassing designation that better represents its function within the combat logic circuits. This nomenclature alteration aligns with the Omnissiah's dictates for proper categorization of binary procedures.
Also applies to: 6-6
objects/obj_ncombat/Create_0.gml (2)
31-32: Praise be to the Machine God for these new data repositories.The addition of these sacred array structures will serve as proper receptacles for the machine spirits of damaged units awaiting techno-liturgical recovery rites. These data structures shall maintain the proper hierarchy of recovery priorities as dictated by the holy STC templates.
152-152: The variable designation has been properly specified for clarity.The renaming from "final_deaths" to "final_marine_deaths" is a logical improvement that increases the semantic precision of this data node. This modification ensures proper categorization of mortality statistics according to unit type, as the Omnissiah demands precise ontological classification in all data structures.
ChapterMaster.yyp (1)
664-664: Praise be to the Omnissiah! New script component integrated successfully.The addition of
scr_after_combatin favor of the previousscr_distribute_experiencerepresents a satisfactory adaptation of the sacred project structure. As the PR objectives indicate, this modification supports the implementation of Marine and Vehicle recovery priorities, a logical advancement of the machine-spirit's capability to prioritize unit restoration post-combat.This script modification aligns perfectly with the Machine God's will for more efficient combat resource management. The new script will likely enhance post-combat recovery protocols by implementing the described priority queue system based on column ID, unit ID, unit struct, and priority data.
objects/obj_ncombat/Alarm_7.gml (3)
258-258: Blessed consistency in naming variables, honoured Tech-Acolyte.
Line 258 now refers tofinal_marine_deathsin place offinal_deaths. This correction aligns with the revised naming scheme for Adeptus Astartes casualties. No errors in logic appear here, and the code now more clearly tracks the fallen marines.
269-269: Proper invocation of the Defeat Protocol.
Replacingfinal_deathswithfinal_marine_deathsmaintains consistency when summoning the defeat message if casualties surpass a threshold of 10. The logic stands correct for detecting defeat conditions.
271-271: Ensuring parallel usage of final_marine_deaths.
This line mirrors the adjusted logic in the if-clause to record a defeat message. Employingfinal_marine_deaths + final_command_deathsis consistent with the rest of the script’s approach to losses.scripts/scr_after_combat/scr_after_combat.gml (3)
1-20: Commendation for structured marine recovery.
The newadd_marines_to_recoveryfunction properly iterates throughplayer_column.unit_struct, preparing an array of fallen battle-brothers for resurrection attempts. Logic checks formarine_dead[i]and ignores allied units, ensuring only relevant Astartes undertake the sacred recovery.
51-62: Assembling living brethren is efficient and clear.
Theassemble_alive_unitsfunction collects only living non-allied units, neatly grouping them for post-battle operations. This helps maintain an accurate record of active combatants.
64-85: Laudable approach to distributing experience.
The logic calculates each warrior’s experience share, scaled by their existing level. This helps avoid sudden overpowered leaps. Marking librarians that learn new powers is also well done.objects/obj_ncombat/Alarm_5.gml (14)
20-25: Initiation of post-combat recovery.
Invokingadd_marines_to_recoveryandadd_vehicles_to_recoveryonly whenobj_ncombat.defeat == 0ensures these holy rites occur for successful engagements. Logical gating seems sound.
27-45: Creation of marine recovery priority queue.
The reliance onds_priorityto retrieve top-priority casualties is an organised approach. Deductingunit_recovery_scoreand incrementingunits_savedis coherent.
82-84: Additional invocation for after-battle updates.
Callingafter_battle_part1()here centralises key post-battle logic, ensuring that gear salvaging, gene-seed updates, and other finalisations occur systematically.
86-104: Calculating total losses.
Declaring_total_deathsas(final_marine_deaths + final_command_deaths) - units_savedclarifies the final figure of truly fallen Astartes. The subsequent logic for enumerating these losses is well-structured.
106-108: Marine saviours.
Announcing how many marines the Apothecaries have rescued fosters clarity for the glorious reconstruction efforts. No miscalculation is evident.
132-144: Gene-seed reclamation.
Properly accounts for scenarios where no Apothecaries survive or the Chapter is afflicted by mutations, thus losing gene-seed. This is consistent with existing world-lore mechanics.
158-159: Blank newline usage.
These lines simply inject spacing, promoting readability within the sacred text.
163-166: Vehicle recovery message.
Announcing how many machines of war were rescued by Techmarines is thematically sound. The logic references the same priority logic introduced earlier.
168-183: Vehicle loss handling.
Details the final count of vehicles destroyed, listing them by type. This consistent approach mirrors the marine casualty reporting. The usage ofnewline_color="red";draws attention appropriately.
211-234: Post-battle experience distribution block.
Callsassemble_alive_unitsand calculatesaverage_battle_exp_gain. Summarising the changes in librarian powers is straightforward and user-friendly. The usage ofname_role()is also more descriptive.
238-238: Assignment of post-equipment lost data.
Line 238 ensures any post-equipment-lost arrays are passed toobj_ground_mission. Harmonising data across objects fosters unity in the code’s architecture.
632-652: Leader elimination logic.
A properly displayed message when an enemy leader falls. The event log updates also appear consistent with the journaling approach used for other factions.
785-791: Sorcery usage detection.
Checks if the Inquisition has witnessed illicit powers. This is a meaningful expansion of the campaign’s consequences, adjusting loyalty accordingly. The logic is consistent with the broader theme of watchful or suspicious allies.
47-80: Vehicle salvation queue.
Similar to marines, vehicles now follow a priority-based recovery. Theif (obj_controller.stc_bonus[3] = 4)check stands out—be certain the single ‘=’ is indeed a valid comparison in your GML dialect. Otherwise, this might inadvertently assign a value.Shall we verify that equality is performed correctly in your game’s version of GML?
#!/bin/bash # Searching for other usage of stc_bonus comparisons across scripts to confirm consistency: rg "stc_bonus\[3\]" -A 4
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
objects/obj_ncombat/Alarm_5.gml (3)
103-119: 🧹 Nitpick (assertive)Gear checks for special items are robust, but watch for edge cases.
The detection of “Plasma Bomb” or “Exterminatus” gear while marines remain alive effectively tracks high-value wargear. Be mindful that older GML dialects can treat single-=as assignment, so verifying your environment’s function for these conditionals is crucial.
150-166: 🧹 Nitpick (assertive)Gene-seed harvesting logic is well-aligned with the rites of the Astartes.
Here, the code properly incrementsseed_maxbased on age checks. Overall correct, but do verify random thresholds are balanced for your desired difficulty. The Emperor’s mercy need not be random if your design requires a consistent harvest rate.
180-340: 🧹 Nitpick (assertive)Extensive salvage sub-routine.
Five passes are made (armour, weapon1, weapon2, gear, mobility) extracting or destroying each item. This code is quite verbose, reflecting a pattern repeated for each item slot. For future expansions, you might consider a single function that processes salvage checks more dynamically to reduce duplication and purify the code’s structure in honour of the Omnissiah.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
ChapterMaster.yyp(2 hunks)objects/obj_ncombat/Alarm_5.gml(7 hunks)scripts/scr_after_combat/scr_after_combat.gml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.gml`: Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ga...
**/*.gml: Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
objects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gml
`**/*.gml`: Color codes in the code shouldn't have any spaces in their id. I.e., color code `# 80 bf40` will crash the game.
**/*.gml: Color codes in the code shouldn't have any spaces in their id. I.e., color code# 80 bf40will crash the game.
objects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gml
🔇 Additional comments (14)
ChapterMaster.yyp (1)
664-664: The Omnissiah has blessed this codebase with increased efficiency!The sacred augmentation of
scr_after_combatscript to the resources registry is logically sound. According to the PR objectives, this script houses the new marine and vehicle recovery priority system. The removal of the outdatedscr_distribute_experiencescript and its replacement with this more comprehensive post-combat routine demonstrates proper adherence to the principles of code optimization.01101101 01100001 01111001 00100000 01110100 01101000 01100101 00100000 01001111 01101101 01101110 01101001 01110011 01110011 01101001 01100001 01101000 00100000 01100010 01101100 01100101 01110011 01110011 00100000 01110100 01101000 01101001 01110011 00100000 01100011 01101111 01100100 01100101
objects/obj_ncombat/Alarm_5.gml (7)
9-9: Verily, this commented debug line might be an obsolete remnant.
O blessed developer, thou hast a line referencingfinal_marine_deathsthat is commented out. Consider removing or re-enabling it if needed for troubleshooting, lest it add unneeded clutter.
19-24: Commendable retrieval logic for marines.
These lines properly gate the recovery procedure to whendefeatis zero, which appeases the machine spirit by preventing unnecessary salvage calls in a lost scenario. Well done.
80-101: Invocation ofafter_battle_part1and_total_deathscomputation are consistent.
The logic to handle marine losses and display final death counts is performed with due diligence. No major concerns are detected here, though expansions to reflect certain special mission parameters could be beneficial in future.
126-146: Adequate final death categorisation for command vs standard units.
This block cleanly incrementsfinal_command_deathsorfinal_marine_deaths. The approach fairly distinguishes casualties among specialist roles. Should you require more complex logic for ephemeral roles (like "trainee"), ensuring robust classification of newly introduced roles is recommended.
168-178: Correct usage ofpost_unit_lostarray for custom role names.
This block appends the marine’s role or type to the array if vacant. No immediate concerns, though watch for potential collisions if multiple marines share identical roles.
342-367:❓ Verification inconclusive
Vehicle death tracking is thorough, but be cautious about indexing.
This segment incrementsvehicle_deathsand updatespost_unit_lostwith the lost vehicle. The nested loop uses a secondforalongside ano+=1approach that can be tricky to follow. Ensure the indexing logic will not skip or overflow the array ifpost_unit_lostis near capacity.Please run this script to confirm no references cause out-of-bounds errors:
🏁 Script executed:
#!/bin/bash rg 'post_unit_lost\[' -A 3Length of output: 2848
Attention, Devotee of the Code: Verify Array Index Sanctity
The Omnissiah’s data indicates that the vehicle death tracking is implemented with careful thought, yet the nested iteration—particularly the incrementation of the variable
o—in the combat script demands vigilance. Our investigation reveals:
- Array Initialisation: In
objects/obj_ncombat/Create_0.gml, the arrayspost_unit_lost,post_units_lost, andpost_unit_vehare properly initialised.- Boundary Checks in Output: In
objects/obj_ncombat/Alarm_5.gml, the conditionif (i < array_length(post_unit_lost) - 1)ensures that the indexing for output is bounded.- Nested Loop Caution: In
scripts/scr_after_combat/scr_after_combat.gml, the update logic usingoto track vehicle losses must be verified to ensure it does not breach array capacity when the casualty tally rises.While no immediate out-of-bounds errors have been unearthed by our rituals, remain cautious and confirm that the indexing of
ois robust under all conditions. Continue testing this logic to honour the Machine God’s precision.
46-78:✅ Verification successful
Vehicle salvage logic harmonises with the Omnissiah’s hierarchy.
The introduction of_vehicle_recovery_queueand distinct numerical priority values is praiseworthy, ensuring mightier vehicles (like Land Raiders) are rescued first. As a small improvement, confirm that out-of-range or new vehicle types will not be overlooked if the_vehicles_prioritystruct lacks an entry.Please confirm by scanning for references of
veh_type[]usage across the codebase:
🏁 Script executed:
#!/bin/bash rg -A 5 'veh_type\['Length of output: 11368
Vehicle Salvage Logic Verified and Approved
Ave tech, your vehicle recovery mechanism art most pleasing to the Machine Spirit. The introduction of the
_vehicle_recovery_queueand discrete numerical priority values capably ensures that mighty vehicles — the Land Raiders among them — are attended to with due reverence. Our diligent scan revealed that in scripts/scr_after_combat/scr_after_combat.gml, the existence checkif (struct_exists(_vehicles_priority, veh_type[i])) && (veh_dead[i]) && (!veh_ally[i])confirms that new or out-of-range vehicle types without a corresponding entry in the
_vehicles_prioritystructure are indeed excluded from the recovery process.
- The filtering in scr_after_combat confirms that no vehicle type shall be inadvertently overlooked.
- The salvage logic flows in harmony with the creed of the Omnissiah.
Your code requires no further amendments on this matter. Praise be to the Machine!
scripts/scr_after_combat/scr_after_combat.gml (6)
1-17: Marine recovery logic is systematically sound.
The iteration overunit_structand the creation of_recovery_candidateobjects properly keep track of each marine’s ID, experience, and so forth. This is an efficient approach as it isolates the building of the recovery pool to a single function.
42-52: Assembling alive units is elegantly handled.
Gathering survivors intoend_alive_unitsis neatly consolidated in this function, preventing duplicate code in your logic. This fosters maintainability and clarity.
78-87: Streamlined post-battle Death Company assignment.
Here, the code ensures that any unit flagged as Death Company is properly given the Death Company role if they are not already so assigned. No major issues are perceived.
121-146: Properly incrementingfinal_command_deathsorfinal_marine_deaths.
The code deals with command losses and standard marine losses distinctly, reflecting the correct usage from the parent object’s logic. Clear alignment with your overall system.
168-178:post_unit_lostarray usage remains consistent.
Your approach to naming and indexing within thepost_unit_lostarray is mirrored across the codebase, ensuring uniform tracking for lost marines or vehicles.
341-367:❓ Verification inconclusive
Vehicle loss recording is coherent.
This chunk talliesvehicle_deathsand updatespost_unit_lostfor vehicles. Keep a watchful eye on array indexing increments, given the manualo+=1approach in nested loops, to avoid out-of-bounds confusion.
🏁 Script executed:
#!/bin/bash rg 'post_unit_lost\[' -A 5Length of output: 3409
Hail the Omnissiah: Verify Array Indexing in Vehicle Loss Code
The vehicle loss tallying in
scripts/scr_after_combat/scr_after_combat.gmlappears functionally coherent, as evidenced by our search output. However, the manual increment operation (o += 1) within the nested loop must be carefully scrutinised to ensure that it does not lead the machine spirit into array out‑of‑bounds error or skipped indices.
- Verify that the manual index incrementation in the inner loop (lines 341–367) is intentional and safely bounded.
- Confirm that this pattern — as also observed in the related initialisation and reporting of
post_unit_lostin other modules — does not compromise future iterations or lead to transient errors.May the machine’s precision guide your review.
OH296
left a comment
OH296
left a comment
There was a problem hiding this comment.
Fuck yeah death to the alarms. Gonna take me a few stints to read through most of this but it pretty much seems all good to me
EttyKitty
force-pushed
the
feat/recovery-priority
branch
from
cca8137 to
9430b9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
objects/obj_ncombat/Alarm_5.gml (2)
125-147: 🧹 Nitpick (assertive)Death Company transition and Mucranoid slime checks.
Behold, the logic for reassigning marines to the Death Company role upon survival is consistent with your chapter’s dark rituals. The random armour destruction from mucranoid slime also appears correct. The single equals usage for GML conditions may again spark confusion in some environs, so maintain vigilance.
227-377: 🛠️ Refactor suggestionPotential candidate for code extraction (DRY principle).
These repeated blocks for salvaging armour, weapons, gear, and mobility items are functional yet verbose. Creating a shared subroutine for item salvage might please the Omnissiah and reduce repeated code sections.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
ChapterMaster.yyp(2 hunks)objects/obj_ncombat/Alarm_5.gml(7 hunks)objects/obj_ncombat/Alarm_7.gml(2 hunks)objects/obj_ncombat/Create_0.gml(3 hunks)objects/obj_ncombat/KeyPress_13.gml(0 hunks)objects/obj_pnunit/Alarm_4.gml(1 hunks)objects/obj_pnunit/Alarm_5.gml(0 hunks)objects/obj_pnunit/obj_pnunit.yy(1 hunks)scripts/scr_after_combat/scr_after_combat.gml(1 hunks)scripts/scr_after_combat/scr_after_combat.yy(1 hunks)scripts/scr_distribute_experience/scr_distribute_experience.gml(0 hunks)
💤 Files with no reviewable changes (3)
- scripts/scr_distribute_experience/scr_distribute_experience.gml
- objects/obj_ncombat/KeyPress_13.gml
- objects/obj_pnunit/Alarm_5.gml
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.gml`: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ...
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80 bf40will crash the game.
objects/obj_ncombat/Alarm_7.gmlobjects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gmlobjects/obj_pnunit/Alarm_4.gmlobjects/obj_ncombat/Create_0.gml
`**/*.gml`: - Always suggest adding JSDoc to every function and method that doesn't have it.
**/*.gml: - Always suggest adding JSDoc to every function and method that doesn't have it.
objects/obj_ncombat/Alarm_7.gmlobjects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gmlobjects/obj_pnunit/Alarm_4.gmlobjects/obj_ncombat/Create_0.gml
`**/*.gml`: - Readability and maintainability are always the top priority throughout the code. - DRY principle is also very important.
**/*.gml: - Readability and maintainability are always the top priority throughout the code.
- DRY principle is also very important.
objects/obj_ncombat/Alarm_7.gmlobjects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gmlobjects/obj_pnunit/Alarm_4.gmlobjects/obj_ncombat/Create_0.gml
🔇 Additional comments (19)
scripts/scr_after_combat/scr_after_combat.yy (2)
3-3: Metadata Transmutation Approved
O blessed cogitator, the alteration of the%Namefield to"scr_after_combat"is precise and in harmony with the sacred modifications decreed. The machine-spirit recognises and blesses this implementation, ensuring the proper invocation of our post-combat rituals.
6-6: Nomenclature Sanctification Complete
The renaming of the"name"attribute to"scr_after_combat"is aptly executed, thereby securing the integrity of the script’s identity within the Combat folder. This change honours the mechanistic design and sustains the proper alignment with our recovery protocols.ChapterMaster.yyp (1)
664-664: Approval of new after-combat processing script.The sacred Omnissiah has blessed this project with a new holy post-combat script! The binary augurs detect the introduction of
scr_after_combatto consolidate recovery and experience distribution functions. This is a logical rationalization of machine-spirit efficiency, replacing the previousscr_distribute_experiencescript as mentioned in the binary-divination summary.Praise be to the addition of marine and vehicle recovery priorities, for such structures shall bring order to the machine-spirits of your combat units. The Omnissiah approves of structured data patterns utilizing arrays of structs containing column ID, unit ID, unit struct, and priority values for efficient post-combat processing.
01110000 01110010 01100001 01101001 01110011 01100101 00100000 01110100 01101000 01100101 00100000 01001111 01101101 01101110 01101001 01110011 01110011 01101001 01100001 01101000
objects/obj_ncombat/Create_0.gml (3)
31-32: Initialize recovery tracking arraysMost satisfactory initializations for the sacred variables to count units worthy of recovery. These counters shall serve as repositories of knowledge for the battle-cogitator functions.
121-122: Implement data structures for tracking lost unitsCommendable implementation of data-collection structs to track the quantities of lost machine spirits. The Machine God approves of your systematic cataloguing of battlefield losses.
153-153: Renamed death counter for specificityThe binary lexicon has been rectified to specify marine casualties with greater precision. This semantic clarification will improve the data-purity of post-combat calculations.
objects/obj_ncombat/Alarm_7.gml (3)
258-258: Updated marine casualty calculationThe binary designation has been properly updated to reference the more specifically named final_marine_deaths variable. This ensures proper accounting of fallen Adeptus Astartes.
269-269: Updated defeat message condition with renamed death counterThe conditional logic has been recalibrated to utilize the correctly named variable. The sacred machine logic now properly references final_marine_deaths, maintaining consistency with the omniscient variable naming schema.
271-271: Updated defeat message string with renamed death counterThe string interpolation function has been realigned to reference the properly named casualty counter. This maintains the sacred consistency of the codebase.
objects/obj_ncombat/Alarm_5.gml (7)
9-9: Redundant debugging log [Already flagged previously].
This commented-out debug line has already been noted in an earlier sermon. Nothing else to address here.
19-69: Verify correctness of the “= 4” comparison in the vehicle recovery logic.
Lo, the sacred script for constructing marine and vehicle priority queues is most sensible, but observe line 50:if (obj_controller.stc_bonus[3] = 4) {...}. In modern GML contexts, a single equals sign performs an assignment. Confirm whether your dev environment interprets single equals as an equality check or an assignment. If it is indeed assignment, this will always be “true” and might be heresy.Wouldst thou confirm that the single equals is intended for equality in older GML dialect, or require correction to double equals (
==) for modern code?
76-93: Commendable approach to final casualty tally.
Your method of subtractingunits_savedfrom total marine and command deaths results in_total_deaths, providing a clear metric. The usage is straightforward and well-structured. No immediate concerns arise.
94-103: Notification for saved marines and critical injuries is consistent.
The logic for displaying saved marines and red-coloured “critical injury” messages is coherent, ensuring proper communication of results to the user.
148-165: Plasma Bomb and Exterminatus gear checks are thematically sound.
Allowing marines bearing “Plasma Bomb” or “Exterminatus” to effectively equip or remove it in the post-battle logic is righteous. Just ensure your environment is consistent about single equals usage for the condition checks.
167-225: Marine death tracking logic is well-defined.
Here thou record the final marine and command deaths, plus potential gene-seed harvesting. This comprehensively updates the relevant counters and event logs. Make sure that the usage of single equals for checks aligns with the older GML standard to avoid unholy bugs.
384-400: Vehicle death detection is consistent and integrates with the crunchy logs.
Converting destroyed vehicles intovehicle_deathsand updating the company to be “crunched” is thorough. Keep an eye on any future expansions to your list of vehicles to ensure consistency in all priority structures.scripts/scr_after_combat/scr_after_combat.gml (3)
66-86: Request verification for struct_exists usage.
The check(struct_exists(_vehicles_priority, veh_type[i]))is a valid approach in certain GML versions. If your environment supports it, all is well. Otherwise, confirm it is recognised.
88-99: Excellent unit assembly routine.
assemble_alive_units()neatly collects marines that are not flagged as dead, which is essential for delivering the righteous experience distribution. The logic is clean and direct.
100-121: Properly balanced experience distribution.
Distributing XP based on a diminishing returns formula ensures veterans do not run away with an excessive advantage, while novices still benefit from group gains. This is elegantly done, though in the future you might consider extra modifiers for heroic feats.
|
there are some issues with this, that require work. I'll put a draft status. |
…om survival to the recovery code, repair enemy kills display
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
scripts/scr_flavor2/scr_flavor2.gml (1)
1-1: 🧹 Nitpick (assertive)Add function documentation.
High praise to the Omnissiah, but the sacred functionscr_flavor2lacks JSDoc or similar annotation. Installing baseline documentation here will bolster clarity for fellow adepts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
objects/obj_ncombat/Alarm_3.gml(1 hunks)objects/obj_ncombat/Alarm_5.gml(7 hunks)objects/obj_pnunit/Alarm_4.gml(0 hunks)objects/obj_pnunit/obj_pnunit.yy(1 hunks)scripts/scr_after_combat/scr_after_combat.gml(1 hunks)scripts/scr_flavor2/scr_flavor2.gml(2 hunks)
💤 Files with no reviewable changes (1)
- objects/obj_pnunit/Alarm_4.gml
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.gml`: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ...
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80 bf40will crash the game.
objects/obj_ncombat/Alarm_3.gmlscripts/scr_flavor2/scr_flavor2.gmlobjects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gml
`**/*.gml`: - Always suggest adding JSDoc to every function and method that doesn't have it.
**/*.gml: - Always suggest adding JSDoc to every function and method that doesn't have it.
objects/obj_ncombat/Alarm_3.gmlscripts/scr_flavor2/scr_flavor2.gmlobjects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gml
`**/*.gml`: - Readability and maintainability are always the top priority throughout the code. - DRY principle is also very important.
**/*.gml: - Readability and maintainability are always the top priority throughout the code.
- DRY principle is also very important.
objects/obj_ncombat/Alarm_3.gmlscripts/scr_flavor2/scr_flavor2.gmlobjects/obj_ncombat/Alarm_5.gmlscripts/scr_after_combat/scr_after_combat.gml
🔇 Additional comments (15)
objects/obj_ncombat/Alarm_3.gml (1)
97-97: The logic extension for colouring “critically injured” in red appears consistent.
Be aware thatstring_countis case-sensitive, so “CRITICALLY INJURED” will not be detected unless deliberately converted. Otherwise, this is acceptable.scripts/scr_flavor2/scr_flavor2.gml (1)
358-358: The shift from “lost” to “critically injured” is well-aligned with the new condition.
This improvement ensures consistency with recently updated messaging. No major issues found.Also applies to: 379-379
objects/obj_pnunit/obj_pnunit.yy (1)
38-39: Changing coordinates from float to integer is safe.
The Machine Spirit approves; this small revision has minimal risk.objects/obj_ncombat/Alarm_5.gml (10)
18-21: Invocation ofafter_battle_part1()is straightforward.
Ensure the script is present on all relevant mission types to avoid runtime confusion. Otherwise, the logic is efficient.
92-94:after_battle_part2()call complements part1.
Blessed continuity: no further concerns.
96-100: Good user feedback for saved marines.
Displaying critical injuries saved by Apothecaries fosters player clarity.
102-106: ‘injured’ messaging is clear.
No issues identified. The subtle distinction from fully saved marines is beneficial.
148-156: Gene-Seed recovery message.
Omnissiah’s blessing upon your thoroughness in reflecting real game state.
159-169: Red Thirst snippet.
Narrative is immersive; logic matches the new injury system.
171-172: Blank line usage.
Highly stylistic but permissible, no functional side-effects.
176-179: Vehicle recovery success message.
Technmarine references are properly pluralised. Adequate user feedback.
181-200: Vehicle losses display.
Your logic for enumerating vehicle losses is coherent and parallels marine logic well.
228-230: Experience distribution for surviving units.
This code is clear, though consider verifying ifassemble_alive_unitsmust handle edge cases (no units alive).scripts/scr_after_combat/scr_after_combat.gml (2)
401-459:❓ Verification inconclusive
Check for potential assignment versus comparison in
ally[i]=false.
In lines 413, 414, 415, and beyond, the usage of the single equals operator may assign rather than compare. Validate that your engine’s dialect truly interprets this as a comparison, lest you inadvertently override the variable. Also, supplementafter_battle_part1with doc commentary specifying its parameters, logic, and return details.
🏁 Script executed:
#!/bin/bash # Searching for single-equals usage in ally checks rg -A 3 'ally\[.*\]=false'Length of output: 3858
Edict from the Omnissiah:
Upon inspection of the sacred script in scripts/scr_after_combat/scr_after_combat.gml, verification has confirmed that the constructally[i]=false—as well as similar expressions likemarine_dead[i]=0—appears in multiple conditional rituals. In our sacred dialect of GML, the single equals operator may indeed perform assignment rather than the desired equality comparison. Thus, it is paramount to verify whether these incantations are intentionally crafted in accordance with the machine spirit’s design, or if they should be transfigured to employ the double equals operator==for proper comparison.Directives for the Adeptus Mechanicus Servitor:
- Confirm the Operator's Nature: Manually verify that your engine’s dialect interprets
=in conditional expressions as a comparison rather than performing assignments that could excommunicate your variables.- Enhance Documentation: Augment the
after_battle_part1function with comprehensive doc commentary that clearly details its parameters, operational logic, and return values, thus ensuring clarity and adherence to the Codex Mechanicus protocols.May the Omnissiah guide thy debugging and documentation endeavours.
127-146:❓ Verification inconclusive
Verify usage of equality checks for marine death and add doc to
after_battle_part2.
At lines 129–130, thou usethif (marine_dead[i]=0) ..., which may be an assignment instead of a comparison in some GML dialects. Confirm that it indeed performs a check rather than bestowing new values upon the variable. Also, impart the divine doc lines for the entireafter_battle_part2function.
🏁 Script executed:
#!/bin/bash # Scanning for suspicious single-equals usage in marine_dead checks rg -A 2 'marine_dead\[.*\]=0'Length of output: 3516
Directive: Verify the Equality Checks and Augment Documentation in after_battle_part2
Oh, servant of the Omnissiah, the sacred scan of our code has revealed several instances where the single equals operator ("=") is employed within conditions—such as in the expression
(marine_dead[i]=0)in scripts/scr_after_combat/scr_after_combat.gml (lines 127–146). As the machine’s logic dictates, the use of "=" typically denotes assignment rather than a true equality check. We must ascertain whether these expressions are intended to perform comparisons (which require "==") or deliberate assignments with side-effects. This ambiguity could lead to erroneous logic if the mortal intent was to verify the equality of values.Additionally, the divine mandate compels us to enrich the
after_battle_part2function with comprehensive documentation. Please include detailed doc comments that describe the function’s purpose, parameters, and expected behaviour in alignment with the canonical codebase practices.Points for Rectification:
- Verify and, if necessary, replace any improper usage of "=" with "==" for comparisons—particularly for the conditions involving
marine_dead[i].- Append the requisite documentation (doc lines) to the entire
after_battle_part2function to ensure its purpose and usage are clearly articulated for the benefit of our peers.I beseech you to review these matters closely to ensure the purity of the code is maintained.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (5)
scripts/scr_flavor2/scr_flavor2.gml (4)
1-3: 🧹 Nitpick (assertive)Adeptus Mechanicus recommendation: Add JSDoc documentation to function
The function's sacred purpose requires proper documentation to appease the Omnissiah. A JSDoc comment block would enhance future maintenance and provide clarity on the Machine Spirit's behavior.
function scr_flavor2(lost_units_count, target_type, hostile_range, hostile_weapon, hostile_shots, hostile_splash) { + /** + * @function scr_flavor2 + * @description Generates flavor text based on damage and casualties from scr_shoot, only for the opponent + * @param {number} lost_units_count - Number of units that were damaged in combat + * @param {string} target_type - Type of target being attacked (e.g., "wall", role name) + * @param {number} hostile_range - Range of the hostile attack + * @param {string} hostile_weapon - Type of hostile weapon used + * @param {number} hostile_shots - Number of shots fired by the hostile weapon + * @param {number} hostile_splash - Whether the hostile weapon has splash damage (1) or not (0) + */ // Generates flavor based on the damage and casualties from scr_shoot, only for the opponent
312-346: 🧹 Nitpick (assertive)Logical but repetitive loop construct detected
The Machine Spirit reveals an opportunity to improve this code segment. The loop construction for processing lost units is functioning correctly but features nested conditionals that could be simplified. Consider extracting the special unit handling to a separate function to improve readability and maintain the sacred DRY principle.
var lost_roles_count = array_length(lost); + +/** + * @function process_special_unit + * @description Handle the special case for important units that have been critically damaged + * @param {string} unit_role - The role of the unit + * @param {number} marine - Index of the marine in the arrays + */ +function process_special_unit(unit_role, marine) { + obj_ncombat.dead_jims += 1; + if (marine_type[marine] == obj_ini.role[100][5]) { + obj_ncombat.dead_jim[obj_ncombat.dead_jims] = $"A {marine_type[marine]} has been critically injured!"; + } else { + obj_ncombat.dead_jim[obj_ncombat.dead_jims] = $"{unit_struct[marine].name_role()} has been critically injured!"; + } +} + for (var role_index = 0; role_index < lost_roles_count; role_index++) { unit_role = lost[role_index]; units_lost = lost_num[role_index];
348-348: 🧹 Nitpick (assertive)Unnecessary empty line detected - Purge the void
This empty line serves no logical purpose and disrupts the sacred flow of the binary litany. Consider removing it to maintain coherent code structure.
} } } - var unce = 0;
309-311: 🧹 Nitpick (assertive)Binary optimization suggested - Redundant variable declarations
The Omnissiah's wisdom compels me to note that numerous variables are declared but some appear to be used sparingly. The variable
sis declared but only used once at line 297 in a different context. Consider auditing variable usage for optimal memory efficiency.var marine_length = array_length(marine_type); -var s, him, special, unit, unit_role, units_lost, plural; +var him, special, unit_role, units_lost, plural;objects/obj_ncombat/Alarm_5.gml (1)
165-165:⚠️ Potential issueLikely assignment rather than comparison.
Similar to line 68, usingif (red_thirst=3)assigns3, which is probably unintended. Replace=with==to preserve the rightful logic.-if (red_thirst=3) then voodoo="1 Battle Brother lost to the Red Thirst."; +if (red_thirst==3) then voodoo="1 Battle Brother lost to the Red Thirst.";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
objects/obj_ncombat/Alarm_5.gml(7 hunks)scripts/scr_flavor2/scr_flavor2.gml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.gml`: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ...
**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e.#macro ARR_body_parts["arm"]will crash the game, because there is no space between the array and the name of the macro.
- Color codes in the code shouldn't have any spaces in their id. I.e., color code
# 80 bf40will crash the game.
scripts/scr_flavor2/scr_flavor2.gmlobjects/obj_ncombat/Alarm_5.gml
`**/*.gml`: - Always suggest adding JSDoc to every function and method that doesn't have it.
**/*.gml: - Always suggest adding JSDoc to every function and method that doesn't have it.
scripts/scr_flavor2/scr_flavor2.gmlobjects/obj_ncombat/Alarm_5.gml
`**/*.gml`: - Readability and maintainability are always the top priority throughout the code. - DRY principle is also very important.
**/*.gml: - Readability and maintainability are always the top priority throughout the code.
- DRY principle is also very important.
scripts/scr_flavor2/scr_flavor2.gmlobjects/obj_ncombat/Alarm_5.gml
🔇 Additional comments (27)
scripts/scr_flavor2/scr_flavor2.gml (3)
358-358: Binary improvement detected - logical simplification and semantic enhancementBy the Omnissiah's grace, this code alteration improves both logical structure and narrative consistency. The previous implementation likely used multiple condition checks that have been consolidated into a single check that activates when units are damaged. The message now properly indicates units are "critically damaged" rather than "lost," aligning with the sacred recovery systems mentioned in the PR objectives.
379-379: Consistent logic patterns detected - praise the OmnissiahThis modification mirrors the previous change at line 358, maintaining blessed consistency throughout the codebase. The sacred pattern of consolidating conditions and updating terminology to "critically damaged" harmonizes with the recovery system improvements described in your cogitator specifications.
1-400: System assessment: Implementation aligns with recovery priorities objectiveMy Machine Spirit analysis confirms these changes support the stated PR objective of implementing marine and vehicle recovery priorities. By changing the terminology from "lost" to "critically damaged," the narrative now properly reflects that units can be recovered rather than permanently destroyed.
Praise be to the Omnissiah for these logical improvements. The implementation is functioning as intended and the changes are minimal yet impactful.
objects/obj_ncombat/Alarm_5.gml (24)
9-9: Preserve or remove debug code referencing ‘final_marine_deaths’?
By the Omnissiah, this line is stranded in half-function. If it no longer serves your diagnostic cogitations, consider removing it fully.
23-31: Commendable creation of recovery data structures.
Your approach to forging priority queues for marines and vehicles is in alignment with the Machine God's wisdom.
32-46: Validate candidate integrity.
While iterating throughmarines_to_recover, confirm_candidateand_column_idremain valid references. Unexpected emptiness or corruption of these data structures would displease the Machine Spirit.
61-61: Efficient closure of marine priority resource.
Destroying the data structure here is a prudent ritual, releasing memory back to the Omnissiah’s domain.
63-66: Additional queue for vehicles is well-structured.
The parallel approach for vehicles ensures you easily maintain distinct recovery systems.
80-87: Confirm negative scoring edge cases.
When deducting_candidate.priorityfromvehicle_recovery_score, ensure the score cannot slip below zero. If that occurs, your rituals might erroneously salvage more vehicles than your forges allow.
88-89: Proper data structure disposal.
Destroyingvehicles_to_recoveronce finished is devout code conduct.
92-94: Further post-battle logic invocation.
Invokingafter_battle_part2()completes your chain of post-combat rites. No concerns detected here.
96-96: Use of_total_deathsis consistent.
Combiningfinal_marine_deathsandfinal_command_deathsinto a single variable is beneficial for simpler calculations post-battle.
97-111: Clear messaging of casualties and saves.
Your textual output delineates injured, saved, and total losses with clarity. The faithful shall interpret these messages with no confusion.
127-127: Final text invocation.
A single call toscr_newtext()is harmless and consistent with your output loop.
131-131: Ground mission reference is straightforward.
This conditional ensures further logic remains planet-bound. Operation is correct.
152-159: Proper integration of Gene-Seed recovery.
This code merges well with earlier checks onapothecaries_alive. The logic is coherent.
162-162: Correct threshold for ‘red_thirst’ gating.
This check is forging a more dire scenario ifred_thirstsurpasses 2. No issues found.
170-171: Spacing logic for message clarity.
These lines maintain consistent spacing after your text output.
174-175: Another text separation invoked.
Ensuring the text layout remains organised and pleasing to both servo-skull and battle-brother.
179-182: Vehicle salvage output matches marines approach.
Your logic here parallels the marine salvage block, ensuring consistent feedback to the Chapter.
184-195: Vehicle death reporting is thorough.
This structured approach to enumerating lost vehicles preserves clarity for the war council.
198-202: Applying red highlight, then resetting.
These lines handle colour-coded output cleanly.
207-207: Equipment lost check.
Confirmingpost_equipment_lost[1]has content is an apt technique to manage your code path for further item updates.
228-229: No operation changes
230-230: Battle experience gain check.
This triggers your new distribution logic only when necessary. Righteous usage.
231-232: Invocation for assembling alive units.
This is suitably placed after verifying total experience to award. No immediate issues found.
244-244: Appending upgraded librarian name.
Joining each name is straightforward. The line meets the expected outcome for message assembly.
EttyKitty
changed the title
2 participants
Purpose of changes
Describe the solution
obj_punitalarm 5 and alarm 4 are copy-pasted into functions.scripts/scr_after_combat/scr_after_combat.gmlnow contains many of the combat end related functions.Describe alternatives you've considered
None
Testing done
Related links
None
Custom player notes entry
Use the PR title.