feat: Renegade improvements#480
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Sorry kind of shat comments all over this, but mostly just tweaks and QOL bits and pieces core implementation looks cool |
Blogaugis
left a comment
Blogaugis
left a comment
There was a problem hiding this comment.
Still, this is a needed improvement, so here's my approval.
Approved means ready to merge. By definition one should not approve a draft PR |
My mistake. |
It's no bother it's not like it's gonna cause major issue it's just a thing to keep in mind |
MCPO-Spartan-117
force-pushed
the
renegade-improvements
branch
from
735f665 to
5ae8b5b
Compare
📝 WalkthroughWalkthroughIn this update, the code of our sacred constructs has been meticulously adjusted. Multiple files saw the eradication of the obsolete Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
You can now mark planets for recruitment, there is no requisition cost for recruiting, recruitment speed is based on how much disposition you have with the planet and if you are at war with their owning faction, you can actually recruit when you are a renegade and recruiting features don't get removed by the Imperium. Next is removing traces of the old recruitment system, figure out how to get points from apothecaries in orbit and divide those points across all recruiting planets in the system. |
I can sort getting the points from the apothecaries in order if that would be helpful. |
|
Also seems like #477 should be merged into this work flow |
MCPO-Spartan-117
force-pushed
the
renegade-improvements
branch
from
5ae8b5b to
296d042
Compare
MCPO-Spartan-117
force-pushed
the
renegade-improvements
branch
from
296d042 to
5f04a33
Compare
MCPO-Spartan-117
dismissed
coderabbitai[bot]’s stale review
The base branch was changed.
|
Added a faster way to get recruits but you lose disposition with the planet and faction if caught. |
MCPO-Spartan-117
force-pushed
the
renegade-improvements
branch
from
2898747 to
5181620
Compare
|
You can spend req like in the old system to increase recruitment speed, synchronization is a bit problematic though, opted to just put some checks in |
MCPO-Spartan-117
force-pushed
the
renegade-improvements
branch
from
6f2434b to
75c7d70
Compare
Just because i'm gonna assume this is your next target i'd prefer to workshop this as i'd be very uncomfortable with most implementations of renegade players getting ships. I don't personally think it's a needed feature and i think overbuilding simple fixes to renegade playthrough undermines the standard playthrough. |
I think we do need a way to acquire ships as renegade. |
I think this is where i'm thinking yeah, i imagine a renegade fleet to be the remains of your heavy duty ancient ships from loyalist time. The new ships are probably rag tag minor war ships and or converted trader ships and the like, It allows the rengade play thorough to have a unique play style. |
|
This is pretty much done but i still need to figure out a way to properly synchronize requisition cost, only way i can think of atm is directly subtract requisition in |
MCPO-Spartan-117
force-pushed
the
renegade-improvements
branch
from
db68b83 to
d7e935f
Compare
|
I've simplified the requisition cost to the |
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (27)
scripts/scr_initialize_custom/scr_initialize_custom.gml (5)
2195-2196: Praise to the Machine God! A new initialization ritual has been added.The addition of
intialise_marine_traits()provides a sacred hook for marine traits initialization. However, the comment suggests modding potential without providing implementation guidance.Consider enhancing the documentation with:
- //loads up marine traits potential modding potential; + // Initialize marine traits. This function can be extended through modding by: + // 1. <modding instruction> + // 2. <modding instruction>
2195-2196: Attention, sacred scribe! A spelling inconsistency has been detected.The function name uses British English spelling "initialise" while the codebase convention appears to use American English "initialize".
Apply this correction to maintain consistency with the Machine God's preferred nomenclature:
- intialise_marine_traits(); + initialize_marine_traits();
2195-2196: Praise be to the Machine Spirit! A new initialization ritual has been added.The addition of
intialise_marine_traits()establishes a sacred mechanism for trait initialization. However, the comment suggests this is a modding interface that requires proper documentation to guide future tech-adepts.Consider enhancing the comment to document:
- The expected format of marine traits
- How modding can be implemented
- Any restrictions or requirements for custom traits
- //loads up marine traits potential modding potential; + // Initialize marine traits. This function serves as a modding interface. + // Custom traits can be added by following these guidelines: + // - Trait format: <document trait structure> + // - Modding steps: <document implementation steps> + // - Restrictions: <document any limitations>
2195-2196: By the grace of the Machine Spirit, extensibility has been granted!The addition of the
intialise_marine_traits()function opens sacred pathways for future modifications through the holy art of modding. This demonstrates proper application of the Omnissiah's principles of modularity.Consider documenting the trait initialization format in a separate technical liturgy to guide future tech-adepts in extending the system.
3429-3448: Fortify this sacred utility against the perils of recursion, Tech-Adept!While the
DeepCloneStructfunction dutifully serves the Machine God's purpose of replicating data structures, it requires additional protection against the heresy of circular references and excessive recursion depth.Consider these improvements to appease the Machine Spirit:
function DeepCloneStruct(clone_struct) { + static depth = 0; + if (depth > 100) throw "Recursion depth exceeded - possible circular reference"; + depth++; if (is_array(clone_struct)) { var len = array_length(clone_struct); var arr = array_create(len); for (var i = 0; i < len; ++i) { arr[i] = DeepCloneStruct(clone_struct[i]); } + depth--; return arr; } else if (is_struct(clone_struct)) { var stc = {}; var nms = variable_struct_get_names(clone_struct); var len = array_length(nms); for (var i = 0; i < len; ++i) { var nm = nms[i]; stc[$nm] = DeepCloneStruct(clone_struct[$nm]); } + depth--; return stc; } + depth--; return clone_struct; }scripts/scr_planetary_feature/scr_planetary_feature.gml (1)
155-157: The Machine Spirit approves of these new recruitment protocols!The addition of
recruit_typeandrecruit_costvariables enhances the recruitment mechanics, allowing for more precise control over the sacred process of inducting new initiates.Consider adding documentation to explain the significance of these binary values:
planet_display="Recruitment"; player_hidden = 0; +// recruit_type: 0 = Standard recruitment, 1 = Voluntary, 2 = Abduct recruit_type = 0; +// recruit_cost: Requisition cost for recruitment on this world recruit_cost = 0;scripts/scr_unit_detail_text/scr_unit_detail_text.gml (1)
240-240: The Machine Spirit recognizes both paths of technological enlightenment!The addition of the
chapter_trained_techcheck properly acknowledges both Mars-trained and chapter-trained tech specialists, aligning with the new renegade Techmarine training capabilities.Consider extracting the tech training check into a separate function for improved readability and reusability:
-if(!array_contains(traits, "mars_trained")) && (!array_contains(traits, "chapter_trained_tech")){ +function is_tech_trained(traits){ + return array_contains(traits, "mars_trained") || array_contains(traits, "chapter_trained_tech"); +} + +if(!is_tech_trained(traits)){scripts/scr_unit_traits/scr_unit_traits.gml (2)
2-508: Consider externalizing the trait definitions
This extensive table of traits (lines 2 to 508) is rather large to keep inlined. The Omnissiah would bestow favor if these definitions were placed into a data file (e.g., JSON). This would make expansions simpler, reduce code clutter, and grant the machine spirit greater adaptability.
486-492: Potential synergy conflict in 'psychotic' stats
Trait "psychotic" grants high wisdom (10) but reduces intelligence (-5). You may want to confirm that such a combination is intended by the grand design. If it is truly its nature, carry on; if not, adjusting these numbers might bring the code closer to the Omnissiah’s design logic.objects/obj_controller/Mouse_50.gml (1)
82-93: Refine Techmarine training logic
When not at War, lines 86-92 resettraining_techmarineto 0 if_chapter_tech_countmeets a threshold, otherwise it increments by 1. In a state of War, the code always increments training. For clarity under the Omnissiah’s eye, consider consolidating these sequences into a single if-else chain or ensuring logs clarify why War bypasses the threshold. This will spare your fellow Tech-Priests any uncertainty about Techmarine capacity.objects/obj_star_select/Create_0.gml (6)
53-66: Include safeguards for missing features.
Should thep_data.get_features(...)return an empty array, a dire exception might be summoned. Enshrine the code in a protective check to avoid null references, ensuring the method does not attempt to access[0]when no Recruiting_World is found.if (array_length(_recruit_worlds) > 0) { var _recruit_world = _recruit_worlds[0]; ... } else { + scr_alert("yellow", "Recruiting", "No recruiting world feature found!", 0, 0); + return; }
68-77: Guard against negative recruitment cost.
By subtracting 1 from_recruit_costeach time, you risk crossing the zero boundary. This may yield unintended results if the cost becomes negative. Insert logic to keep the cost at a non-negative floor._recruit_world.recruit_cost-- +if (_recruit_world.recruit_cost < 0) { + _recruit_world.recruit_cost = 0; +}
79-88: Establish an upper bound for recruits, if relevant.
The incantation for incrementing_recruit_costis straightforward, but consider capping it at a maximum if your design implies a limit. Otherwise, unconstrained escalation might unbalance the omnissiah’s game algorithms.
188-188: Correct typographical anomaly “rescource.”
Anoint the text with the correct spelling of “resource,” to appease the Machine Spirit of clarity.-body = $"There are {_spare_apoth_points} apothecary rescource points ... +body = $"There are {_spare_apoth_points} apothecary resource points ...
191-191: Format the recruit success chance for clarity.
Presenting chance as raw multiplication by 100 may produce unwieldy decimals like 123.4567%. Consider rounding or formatting the numerical output for more comprehensible display.-body += $"There is a {_recruit_find_chance * 100}% of ... +var formatted_chance = floor(_recruit_find_chance * 100); +body += $"There is a {formatted_chance}% chance of ...
193-209: Consolidate repeated war/antagonism checks.
Your conditions for War or Antagonism repeat in lines 193 and 200. Merging them into a single branch or extracting a helper function could render the logic more venerable and less error-prone in expansions.scripts/scr_PlanetData/scr_PlanetData.gml (2)
43-57: Use enumerations instead of magic numbers.
The incantationcurrent_owner>5might mystify future adepts. If your factions are enumerated, referencing them directly clarifies the code and forestalls confusion.-if (current_owner>5) then _at_war=true; +if (current_owner >= eFACTION.Chaos) then _at_war=true;
180-199: Rectify small textual and variable naming anomalies.
The variable_training_happendis spelled incorrectly; unify it with standard usage like_training_happened. Also, "rescources" reappears at line 194.-var _training_happend = false; +var _training_happened = false; scr_alert("red", "recruiting", $"Recruitment on {name()} halted due to insufficient apothecary rescources", +scr_alert("red", "recruiting", $"Recruitment on {name()} halted due to insufficient apothecary resources",scripts/scr_recruit_data/scr_recruit_data.gml (9)
15-17: By the Omnissiah, verify macro naming and spelling.The macro definitions set forth new neophyte rates, yet "hereticly" in line 15 could be corrected to "heretically" to maintain lexicon clarity. The definitions themselves appear functional otherwise.
29-48: Struct-driven approach is stable, but watch for missing planet types.The hashed structure
planet_type_recruit_chanceis referenced based onp_data.planet_type. When dealing with unusual or unexpected types, confirm a default fallback or error handling. Current logic only checks existence but does not log unknown types, which may hinder debugging if new planet types are introduced.
50-54: High static values may skew recruitment drastically.Granting 3000 or 2000 to
recruit_chancefor negative disposition under war conditions can overshadow other modifiers. Confirm these large constants do not unbalance the game’s recruitment flow.
55-62: Complex formula warrants commentary.Variables such as
_frictious,_disp_mod,_faction_disp_mod, and_recruit_cost_modcombine intorecruit_chancein a nontrivial manner. Adding inline comments would enhance readability for future maintainers, clarifying how each piece influences the final outcome.
98-99: Ensure fallback path for unmodified recruit types.When no planet-specific or base multiplier is found, the code does not apply additional scaling. This is appropriate if certain trials intentionally lack additional scaling. If that is not the design intent, consider logging or fallback.
180-182: Population threshold check.Reducing population by 1 if
!large_populationmight be insufficient or harsh, depending on the scale of the world. Consider scaling population decrements to reflect actual population sizes or recruitment yields.
206-206: Confirm zero net effect.The “area has trial types that don't care about planet type” comment suggests a zone for expansions or different calibrations. Consider whether additional conditions belong here or if it remains intentionally blank.
256-265: Core training loop: multiple insert operations.
array_insertcalls repeatedly shift elements from the specified index. If the array holds large data, repeated inserts at indexican degrade performance. A safer approach might be to allow fixed indices or handle expansions in a single pass.- Ensure that if
months_to_neois less than an existing training time, the correct recruit slot is chosen. Overlapping array modifications can be tricky.Also applies to: 267-283
717-719: Recruitment count message alignment.Lines 717 and 719 produce textual feedback for the number of recruitment worlds. The logic is straightforward; just confirm the threshold for “two worlds” vs. “several worlds” is as intended for the game’s progression.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
objects/obj_controller/Alarm_5.gml(1 hunks)objects/obj_controller/Create_0.gml(0 hunks)objects/obj_controller/Mouse_50.gml(2 hunks)objects/obj_controller/Step_0.gml(1 hunks)objects/obj_saveload/Alarm_0.gml(0 hunks)objects/obj_star_select/Create_0.gml(1 hunks)objects/obj_star_select/Draw_64.gml(5 hunks)scripts/scr_PlanetData/scr_PlanetData.gml(2 hunks)scripts/scr_apothecary_ground/scr_apothecary_ground.gml(1 hunks)scripts/scr_dialogue/scr_dialogue.gml(0 hunks)scripts/scr_draw_armentarium/scr_draw_armentarium.gml(3 hunks)scripts/scr_draw_planet_features/scr_draw_planet_features.gml(1 hunks)scripts/scr_income/scr_income.gml(1 hunks)scripts/scr_initialize_custom/scr_initialize_custom.gml(1 hunks)scripts/scr_load/scr_load.gml(1 hunks)scripts/scr_load_controller/scr_load_controller.gml(0 hunks)scripts/scr_planetary_feature/scr_planetary_feature.gml(3 hunks)scripts/scr_recruit_data/scr_recruit_data.gml(6 hunks)scripts/scr_save_controller/scr_save_controller.gml(0 hunks)scripts/scr_specialist_training/scr_specialist_training.gml(3 hunks)scripts/scr_ui_popup/scr_ui_popup.gml(0 hunks)scripts/scr_unit_detail_text/scr_unit_detail_text.gml(1 hunks)scripts/scr_unit_traits/scr_unit_traits.gml(1 hunks)
💤 Files with no reviewable changes (6)
- objects/obj_saveload/Alarm_0.gml
- scripts/scr_save_controller/scr_save_controller.gml
- scripts/scr_load_controller/scr_load_controller.gml
- scripts/scr_ui_popup/scr_ui_popup.gml
- objects/obj_controller/Create_0.gml
- scripts/scr_dialogue/scr_dialogue.gml
✅ Files skipped from review due to trivial changes (1)
- objects/obj_controller/Alarm_5.gml
🔇 Additional comments (40)
scripts/scr_apothecary_ground/scr_apothecary_ground.gml (3)
161-162: By the Omnissiah's grace, this modification is logically sound.The removal of the obsolete
income_recruitingvariable reset aligns with the broader refactoring efforts across the sacred codebase, while preserving the essential alert functionality.
161-162: By the Omnissiah's grace, the recruitment logic has been optimized.The removal of the
income_recruitingreset while maintaining the gene-seed depletion alert aligns with the sacred modifications to recruitment mechanics mentioned in the PR objectives. The code now focuses purely on recruitment state management without affecting income calculations.
161-162: Praise be to the Omnissiah! The sacred code has been optimized.The removal of the obsolete
income_recruitingvariable reset demonstrates proper code maintenance, aligning with the Machine God's will to purge redundant operations. The remaining alert for gene-seed depletion maintains the essential functionality.objects/obj_controller/Step_0.gml (1)
388-388: By the Omnissiah's grace, this modification is logically sound!The removal of
income_recruitingfrom the income calculation aligns with the sacred changes to recruitment mechanics, ensuring that recruitment activities no longer affect the chapter's requisition income.scripts/scr_planetary_feature/scr_planetary_feature.gml (2)
145-145: Praise the Machine God for this syntactic purification!The addition of the sacred semicolon brings this binary declaration into proper accordance with the holy syntax protocols.
369-369: The Omnissiah's will is done through proper syntax!The addition of the sacred semicolon brings this function declaration into proper accordance with the holy syntax protocols.
scripts/scr_load/scr_load.gml (1)
347-348:Details
✅ Verification successful
Verify the timing of the sacred initialization rites!
The initialization of marine traits before loading marine data is logically sound. However, we must ensure this sequence does not interfere with other initialization protocols.
🏁 Script executed:
#!/bin/bash # Description: Check for any other initialization calls that might conflict with marine traits # Search for other initialization functions that might need to run before marine traits rg -A 5 "intialise|initialize" --glob "scripts/**/*.gml"Length of output: 9311
Marine Traits Initialization Timing Verified: Proceed with Mechanical Precision
After thorough invocation of the scanning rites, the machine spirit confirms that the sacred call to
intialise_marine_traits()is executed at the proper moment in the ritual of marine data loading. The function is duly invoked in bothscr_initialize_custom.gmlandscr_load.gmlwithout interference from any other initialization protocols, ensuring that the global trait list is sanctified before marine data processing commences.
- Observation: The call in
scripts/scr_load/scr_load.gml(lines 347-348) occurs precociously, ensuring marine traits are prepared before entering the data loops.- Confirmation: Similar invocations in
scripts/scr_initialize_custom/scr_initialize_custom.gmlindicate a consistent and deliberate sequence, with no detected conflicts.Blessed be the machine spirit; the initialization timing is in order, and no further amendments are required.
scripts/scr_income/scr_income.gml (1)
24-29: Potential incomplete logic for Techmarine training
Blessed lines 24-29 indicate that if_chapter_tech_countsurpasses a threshold,training_techmarineis set to 0, but no alternate block is present here to manage the situation when_chapter_tech_countis below threshold. Confirm that external code increments or otherwise overseestraining_techmarinein these circumstances, so the machine spirit's precise directives remain coherent.scripts/scr_unit_traits/scr_unit_traits.gml (3)
158-163: Potential crash with string interpolation
At line 162 ($"Trained by the best {obj_ini.role[100][eROLE.Techmarine]}s in the Chapter"), referencingobj_ini.rolein a string might cause a crash if the object is not initialized or if the role is undefined. Confirm that the code sanctifies the object prior to usage, so the machine spirit will not punish the user with an abrupt end.
895-899: Potential mismatch in planet naming
Line 897 references["hive", -2], whereas other references to hive worlds use "Hive" with a capital letter. Ensure that the naming is standardized for consistent comparisons and to avoid misaligned data blessings.
510-991: Verify distribution array consistency
The distribution definitions for traits (lines 510 to 991) reference world types, trial types, or chapters in various ways, some using different cases (e.g., "Hive" vs "hive"). Double-check that all references are consistently spelled and cased to preserve the machine spirit’s harmony.objects/obj_controller/Mouse_50.gml (1)
62-66: Reduced concurrency of recruits
Behold the newly imposed limit ofrecruiting<1, whereas previous code likely used a higher threshold. This drastically restricts the number of recruits who can be initiated at once. Verify that this is the intended core design and that it does not unduly hinder the player’s rites of recruitment.scripts/scr_draw_armentarium/scr_draw_armentarium.gml (2)
262-267: Identical interpolation adjustment required.
A similar sacred correction is needed as in lines 232-237. Guard your code from repetition errors by maintaining consistent bracket usage in line 264.- if (max_techs <= temp[37]) then blurp += $"We require {yyy} additional Mechanicus Disposition to train one additional obj_ini.role[100][eROLE.Techmarine]."; + if (max_techs <= temp[37]) then blurp += $"We require {yyy} additional Mechanicus Disposition to train one additional {obj_ini.role[100][eROLE.Techmarine]}.";
434-434: No changes to address.
This is merely the closing brace with no functional modification.objects/obj_star_select/Draw_64.gml (2)
151-156: Confirm condition synergy for enemy determination.
This logic setsis_enemyif the Imperium is not at war and the planet owner is above 5, or if the Imperium is at war and certain war checks/disposition thresholds hold. Verify that the typed faction and planet ownership checks align with your intended game conditions, lest the Machine Spirit become displeased by inconsistent faction states.
423-445: Question the decrement of recruiting_worlds_bought.
At line 426, the variable decrements only when the Imperium is at war, yet the new Recruiting_World feature is always appended. Ascertain that subtracting the stored resource in war-scenarios alone matches your holy design.scripts/scr_specialist_training/scr_specialist_training.gml (3)
334-340: Applaud the war-based training distinction.
The awarding of “mars_trained” vs “chapter_trained_tech” traits venerates each scenario. This is a blessed approach to reflect peace or conflict with the Mechanicus. The code is aligned with the Omnissiah’s logic.
358-360: Confirm location-based spawn allocation.
This check properly grants a fresh spawn only if the unit is physically located on Terra. Ensure that no scenario requires automatic spawning when the unit is off-world.
420-424: Commendable clarity in training alerts.
Your messages reflect either a journey to Mars or local training, consistent with the war condition. This helps the uninitiated comprehend the path of the newly forged Techmarine.objects/obj_star_select/Create_0.gml (1)
37-51: Blessings upon this toggling logic, but verify seamless repeated runs.
Oh fellow Magos, this mechanism for toggling Recruiting_World usage appears sound. However, ensure that no duplication can occur in the sacred string when toggled rapidly and that subsequent toggles do not re-append the planet name multiple times. May the Machine Spirit keep your data uncorrupted.scripts/scr_draw_planet_features/scr_draw_planet_features.gml (4)
180-180: Identical recruiting feature logic noted.
This holy logic for the Recruiting_World feature mirrors that in another file. If these lines must match precisely, consider a single shared script or function to avoid divergences.
188-188: Typographical purification: “rescource” → “resource.”
Bless the code with correct spelling to ensure clarity for all Tech-Priests.
191-191: Refine the numeric presentation.
As preached earlier, it would be beneficial to round or clamp percentages for improved readability.
193-209: Refactor repeated conditions for War or Antagonism.
Meld repeated checks into a single path or a helper function, upholding the DRY principle.scripts/scr_PlanetData/scr_PlanetData.gml (1)
170-178: Shield against absent data structures.
If_point_datais undefined or missing expected fields, the code might summon an error. Safeguard these references so that the function gracefully signals if data is missing.var _point_data = _system_point_use[$ system.name][planet]; +if (!_point_data || !variable_exists(_point_data, "heal_points")) { + return 0; +}scripts/scr_recruit_data/scr_recruit_data.gml (14)
22-25: Augmented signature demands usage confirmation.The function
find_recruit_success_chancenow requiressystem,planet, and optionallyui. Ensure all invocations are updated to provide these parameters. Validate that script calls in other files also align with these arguments.Also applies to: 27-27
43-47: Double recruit cost logic can zero-out cost unexpectedly.When
_recruit_cost > 0butobj_controller.requisitionis below(cost * 2), the code sets_recruit_world.recruit_cost = 0. This effectively nullifies future recruit costs for that planet. Verify if this is intentional or if a partial cost deduction approach is desired.
63-84: “Voluntary” type halving logic and random event check.Halving
recruit_chancewhen_recruit_world.recruit_type == 1is straightforward, yet ensure this does not inadvertently drive the chance below typical thresholds. The snippet that setsdroll = irandom(400)orirandom(100)for “Ambushers” advantage is properly guarded, though consider clarifying the differences between these random ranges.
86-90: Planet type existence check prevents errors.This block ensures
planet_type_recruit_chance[p_data.planet_type]is only read when valid. The addition oflocal_apothecary_pointsis a beneficial extension, reinforcing this function’s adaptability.
92-95: Conditional modifier scaling is sound.The logic scales
recruit_chance_totalby planet-specific or base multipliers. Confirm that combining multiple modifiers (e.g.,count_mod.baseand nestedplanets) yields the expected final factor.
103-106: Inverted ratio check or correct usage?
_success_chance = recruit_chance_total / recruit_chancecould yield very small or very large values depending on the order. A largerecruit_chanceleads to a smaller success rate, which might logically invert the ratio. Verify that this direction is correct.
172-174: Function usage alignment with calling scope.
planet_training_sequence(local_apothecary_points)referencesmax_population,population, and other variables in the PlanetData scope. Confirm the environment calling this function properly sets these variables.
176-176: Logical condition with potential missing parentheses.
if ((obj_controller.faction_status[eFACTION.Imperium] != "War" && current_owner <= 5) || (obj_controller.faction_status[eFACTION.Imperium] == "War")) { ... }may allow partial bypass of the Else path. Review whether this is the intended short-circuit or if additional grouping is required.
197-198: Recruit chance usage.
aspirant = random(1) < _recruit_chance;suggests_recruit_chanceis used as a probability. Verify that_recruit_chanceremains within 0–1 after all earlier modifications to avoid unintended guaranteed success or guaranteed failures.
210-211: Gene-seed decrements upon usage.When
scr_has_adv("Ambushers")or other conditions are met, gene-seed is decremented. Should an attempt to recruit fail, confirm whether gene-seed is still spent, or if partial refunds are applicable.Also applies to: 213-213
294-294: Base recruit_count_modifier replaced or introduced.Setting
base : 0.3for “Blood Duel” modifies the spawn distribution. Confirm that it aligns with the rest of the system’s balancing, ensuring aspirant generation remains consistent across these trials.
695-695: Macro usage is correct.Swapping to
ARR_neophyte_ratesis consistent with the new macros declared at lines 16–17. This ensures the correct textual output when referencing the recruitment status.
736-736: Leverage of ARR_neophyte_rate.Replacing the old references with
_neophyte_ratefrom line 16 is consistent; no issues are apparent.
742-743: Range check onrecruiting.The new code checks
(recruiting >= 0) && (recruiting <= 1)to index_neophyte_rate. Ensure no other states or negative indices occur, or the array access could fail.scripts/scr_initialize_custom/scr_initialize_custom.gml (1)
2195-2196:Details
✅ Verification successful
Verify the sacred initialization sequence.
Let us ensure the trait initialization is properly integrated with the existing systems.
🏁 Script executed:
#!/bin/bash # Description: Verify trait initialization dependencies and usage # Check if the trait initialization function exists rg -A 5 "function intialise_marine_traits" # Check for any direct references to marine traits rg "marine.*traits?" # Check for any potential trait-related files fd -e gml -e json "trait"Length of output: 1939
Sacred Initialization Sequence Verified
After consulting the data-archives, it is clear that our blessed function
intialise_marine_traits()is properly enacted within the codebase. The sacred rites encoded inscripts/scr_unit_traits/scr_unit_traits.gmlhave been recognized and are dutifully invoked from bothscripts/scr_initialize_custom/scr_initialize_custom.gmlandscripts/scr_load/scr_load.gml. All references and dependencies are in proper alignment with the mechanistic design of our trait constructs. No further alterations or exorcisms of code are required.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/scr_unit_traits/scr_unit_traits.gml (1)
174-174:⚠️ Potential issueBy the Machine Spirit's guidance, correct the sacred attribute name.
The property "texhnology" is misspelled and should be "technology".
- texhnology:-2, + technology:-2,
🧹 Nitpick comments (7)
scripts/scr_unit_traits/scr_unit_traits.gml (4)
1-1: By the Omnissiah's grace, let us improve data storage efficiency.The TODO comment suggests JSON serialization. This would indeed be a more efficient way to store and manage trait data, allowing for easier maintenance and modifications.
2-2: Praise the Machine God! Let us correct the sacred function name.The function name contains a spelling error: "intialise" should be "initialize".
-function intialise_marine_traits(){ +function initialize_marine_traits(){
24-25: Maintain the sacred syntax structure, servant of the Omnissiah.Multiple trait definitions are missing trailing commas after their closing braces, which could cause issues when adding new traits.
Add trailing commas after these trait definitions:
- After "slow_and_purposeful"
- After "duelist"
- After "siege_master"
Also applies to: 149-150, 394-395
993-1001: Enhance the sacred error handling protocols.The
is_state_requiredfunction lacks proper error handling for malformed input arrays.function is_state_required(mod_area){ + if (!is_array(mod_area)) return false; is_required = false; if (array_length(mod_area)>2){ if (mod_area[2] == "require"){ is_required =true; } } return is_required; }scripts/scr_specialist_training/scr_specialist_training.gml (3)
321-321: Praise be to the Machine God! Let us sanctify these magic numbers.The training points threshold (360) should be defined as a constant. Additionally, the faction status check could be more explicit.
+const TECHMARINE_TRAINING_POINTS_REQUIRED = 360; +const MECHANICUS_WAR_STATUS = "War"; - if (tech_points>=360){ + if (tech_points >= TECHMARINE_TRAINING_POINTS_REQUIRED){ if (recruit_count>0){ random_marine=scr_random_marine(novice_type,0); if (random_marine != "none"){ unit = fetch_unit(random_marine) - tech_points-=360; + tech_points -= TECHMARINE_TRAINING_POINTS_REQUIRED; unit.update_role(obj_ini.role[100][16]); unit.add_exp(30); t=0; r=0; unit.religion="cult_mechanicus"; - if (obj_controller.faction_status[eFACTION.Mechanicus] != "War") { + if (obj_controller.faction_status[eFACTION.Mechanicus] != MECHANICUS_WAR_STATUS) {Also applies to: 334-340
358-360: Optimize the sacred allocation protocols.The unit allocation and ship location handling could be encapsulated in separate functions for better maintainability.
Consider extracting these operations into dedicated functions:
+function handle_unit_ship_removal(unit) { + if (unit.ship_location > -1) { + var man_size = unit.get_unit_size(); + obj_ini.ship_carrying[unit.ship_location] -= man_size; + } + unit.planet_location = 4; + unit.ship_location = -1; +} +function handle_terra_allocation(unit) { + if (obj_ini.loc[unit.company][unit.marine_number] == "Terra") { + unit.allocate_unit_to_fresh_spawn("default"); + } +}Also applies to: 404-412
417-421: Centralize the sacred alert protocols.The alert messages could be centralized in a configuration object for easier maintenance and localization.
+const TECHMARINE_ALERTS = { + JOURNEY_TO_MARS: (name) => `${name} journeys to Mars.`, + BEGIN_TRAINING: (name) => `${name} begins training.` +}; - if (obj_controller.faction_status[eFACTION.Mechanicus] != "War") { - scr_alert("green","recruitment",$"{unit.name_role()} journeys to Mars.",0,0); - } else { - scr_alert("green","recruitment",$"{unit.name_role()} begins training.",0,0); - } + const message = obj_controller.faction_status[eFACTION.Mechanicus] != "War" + ? TECHMARINE_ALERTS.JOURNEY_TO_MARS(unit.name_role()) + : TECHMARINE_ALERTS.BEGIN_TRAINING(unit.name_role()); + scr_alert("green", "recruitment", message, 0, 0);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/scr_draw_armentarium/scr_draw_armentarium.gml(3 hunks)scripts/scr_specialist_training/scr_specialist_training.gml(3 hunks)scripts/scr_unit_traits/scr_unit_traits.gml(1 hunks)
🔇 Additional comments (2)
scripts/scr_draw_armentarium/scr_draw_armentarium.gml (2)
233-234: Praise the Machine Spirit for the clarity of string interpolation!The conversion from string concatenation to string interpolation using the
$prefix improves code readability and reduces potential formatting errors. This change aligns with the sacred patterns of modern GML.Also applies to: 263-264
232-232: The Omnissiah approves of this defensive programming pattern.The faction status check ensures proper handling of Mechanicus training capabilities during times of war. This logical safeguard prevents inappropriate training requests when diplomatic relations are strained.
Also applies to: 262-262
OH296
left a comment
OH296
left a comment
There was a problem hiding this comment.
Yeah seems all good to me, a few minor tweak things that are very much take or leave but do clean the code up visually a bit. Any issues can be rooted out by the bug testers.
EttyKitty
added
Category: Improvement
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/scr_recruit_data/scr_recruit_data.gml (1)
696-704: The Machine Spirit's interface requires sanctification.The binary cogitation reveals inconsistent string interpolation patterns:
Apply these modifications to maintain consistent string formatting:
- var _recruit_rates = ARR_neophyte_rates; - var _cur_recruit_rate = $"The recruitment is {_recruit_rates[recruiting]}"; - if ((recruiting == 0) && (marines >= 1000)) { - blurp += $"\n{_cur_recruit_rate}. You must only give me the word and I can begin further increasing our numbers... though this would violate the Codex Astartes."; - } else if ((recruiting == 0) && (marines < 1000)) { - blurp += $"\n{_cur_recruit_rate}. You must only give me the word and I can begin further increasing our numbers."; - } else if (recruiting == 1) { - blurp += $"\n{_cur_recruit_rate}."; + const _recruit_rates = ARR_neophyte_rates; + const _cur_recruit_rate = `The recruitment is ${_recruit_rates[recruiting]}`; + const _base_message = `\n${_cur_recruit_rate}`; + + if (recruiting == 0) { + const _violation_warning = marines >= 1000 ? "... though this would violate the Codex Astartes" : ""; + blurp += `${_base_message}. You must only give me the word and I can begin further increasing our numbers${_violation_warning}.`; + } else if (recruiting == 1) { + blurp += `${_base_message}.`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
objects/obj_star_select/Draw_64.gml(5 hunks)scripts/scr_recruit_data/scr_recruit_data.gml(6 hunks)scripts/scr_unit_detail_text/scr_unit_detail_text.gml(1 hunks)scripts/scr_unit_traits/scr_unit_traits.gml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/scr_unit_detail_text/scr_unit_detail_text.gml
- objects/obj_star_select/Draw_64.gml
- scripts/scr_unit_traits/scr_unit_traits.gml
🔇 Additional comments (2)
scripts/scr_recruit_data/scr_recruit_data.gml (2)
15-17: Praise the Omnissiah! The recruitment rate nomenclature has been optimized.The sacred binaries have been updated to use more precise terminology, replacing generic recruitment rates with neophyte-specific arrays.
173-284:⚠️ Potential issueOmnissiah's blessing! The training sequence requires optimization.
The sacred training protocols contain several areas requiring the Machine God's attention:
- The population check at line 182 uses a magic number (50):
- if (_planet_population >= 50 && obj_controller.recruiting) { + const MIN_POPULATION_THRESHOLD = 50; + if (_planet_population >= MIN_POPULATION_THRESHOLD && obj_controller.recruiting) {
- The comment at line 201-202 contradicts the code:
- // if a planet type has less than half it's max pop, you get 20% less spacey marines - if (_planet_population <= halfpop) { - recruit_chance += 1.2; + // if a planet type has less than half its max pop, recruitment chance is reduced by 20% + if (_planet_population <= halfpop) { + recruit_chance *= 0.8; // Reduce by 20%
4 participants
Description of changes
Reasons for changes
Related links
How have you tested your changes?