feat: Centralise specialists stat requirements#500
feat: Centralise specialists stat requirements#500EttyKitty merged 4 commits intoAdeptus-Dominus:mainfrom
Conversation
📝 WalkthroughWalkthroughThe updates to the repository have meticulously aligned the structure of the 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
scripts/scr_specialist_training/scr_specialist_training.gml (3)
2-60: Consider replacing implicit global references and improving naming for clarity, O blessed Omnissiah.
The functionspecialistfunctusesexperienceandrole()without local parameters. This can obscure the Machine God's flow of data. Renaming the function to something more descriptive (e.g.,scr_specialist_tooltip) and passing required variables explicitly would better reveal the arcane logic.Here is a possible rename diff:
-function specialistfunct (specialist, req_exp) { +function scr_specialist_tooltip (specialist, req_exp, current_exp, current_role) { // example internal updates to reference current_exp / current_role // instead of implicit variables }
63-151: Purge grammatical impurity in the apothecary alert message.
Line 147 spells "traning" instead of "training" in the alert that is shown when no marines are available. Rectifying this upholds the purity of the code’s text strings.- scr_alert("red","recruitment","No marines available for apothecary traning",0,0); + scr_alert("red","recruitment","No marines available for apothecary training",0,0);
154-232: All hail code modularity, though further unification may benefit the Chaplain function.
These lines effectively manage Chaplain recruits, but the logic partially mirrors Apothecary processes. Consider extracting shared segments into a helper function that handles training checks and recruit selection to anoint the code with dryness.scripts/scr_draw_management_unit/scr_draw_management_unit.gml (1)
146-172: Iterate dynamically over the entire spec_train_data array for future expansion.
The loop currently hardcodesfor (var s=0; s<=3; s++), which may lead to out-of-bounds or overlooked expansions if more specialists are introduced. Summon a dynamic iteration, O steward of the code, to maintain synergy with a variable number of specialists.-for (var s=0; s<=3; s++){ +for (var s=0; s<array_length(obj_controller.spec_train_data); s++){objects/obj_controller/Alarm_5.gml (1)
188-190: Summon the Techmarine training call if desired.
Rejoice in the modular design for specialist training. However, the Techmarine functiontechmarine_training()is not invoked here. If the Techmarine’s anointing is not omitted by design, consider adding the missing function call to complete the holy quartet of specialized roles.apothecary_training(); chaplain_training(); librarian_training(); +techmarine_training();scripts/is_specialist/is_specialist.gml (1)
272-287: Praise to the Machine Spirit, but debug messages should be purged for production.The debug messages aid in diagnosing the sacred machine spirit's behavior during development, but may impact performance in production.
Consider wrapping debug messages in a debug flag:
-show_debug_message($"{_stat_val}, {unit.name()} one"); -show_debug_message($"{_stat_val}, {unit.name()} two"); -show_debug_message($"worked"); +if (global.debug_mode) { + show_debug_message($"{_stat_val}, {unit.name()} one"); + show_debug_message($"{_stat_val}, {unit.name()} two"); + show_debug_message($"worked"); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ChapterMaster.yyp(2 hunks)objects/obj_controller/Alarm_5.gml(1 hunks)objects/obj_controller/Create_0.gml(1 hunks)scripts/is_specialist/is_specialist.gml(2 hunks)scripts/scr_draw_management_unit/scr_draw_management_unit.gml(3 hunks)scripts/scr_load_controller/scr_load_controller.gml(1 hunks)scripts/scr_marine_struct/scr_marine_struct.gml(1 hunks)scripts/scr_save_controller/scr_save_controller.gml(1 hunks)scripts/scr_specialist_training/scr_specialist_training.gml(1 hunks)scripts/scr_specialist_training/scr_specialist_training.yy(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- scripts/scr_specialist_training/scr_specialist_training.yy
- scripts/scr_marine_struct/scr_marine_struct.gml
🔇 Additional comments (6)
scripts/scr_specialist_training/scr_specialist_training.gml (1)
312-433: Verify partial training logic for Techmarines, oh caretaker of code.
In lines 384–398, the Techmarine function utilizesspec_train_data[1]for the lower threshold scenario. Double-check that[1]truly belongs to Techmarine. If this references Librarian data, it could corrupt Techmarine initialization.Do you wish to confirm that index 1 is correct or should we unify it with index 0 for consistency?
scripts/scr_draw_management_unit/scr_draw_management_unit.gml (1)
239-239: Divine code clarity is maintained here.
No apparent issues in the text-color rendering logic for HP. The calls look properly aligned with standard usage.scripts/is_specialist/is_specialist.gml (1)
257-258: By the Omnissiah's grace, this syntax modification improves code clarity.The change from array access
search_conditions[$ "stat"]to dot notationsearch_conditions.statenhances readability while maintaining identical functionality.scripts/scr_save_controller/scr_save_controller.gml (1)
184-185: The Machine Spirit approves of this data persistence ritual.The specialist training data is now properly saved to the sacred INI file, ensuring it persists between sessions.
ChapterMaster.yyp (2)
613-613: Praise be to the Omnissiah! A new script has been consecrated.The addition of
scr_specialist_trainingscript resource is a logical step in centralizing the sacred rites of specialist training, aligning with the Machine God's preference for order and structure.
61-61: Blessed reorganization of the binary hierarchy detected.The addition of the "allies" folder under Marine Viewer sprites represents a proper categorization of visual assets, following the sacred patterns of data organization.
OH296
changed the title
EttyKitty
added
Type: Refactor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
scripts/scr_draw_management_unit/scr_draw_management_unit.gml (1)
438-441:⚠️ Potential issueWarning: Potential logic error in force tool evaluation detected!
The condition for weapon comparison contains a logical inconsistency:
- or (temp[110]!=unit.weapon_one()) + or (temp[110]!=unit.weapon_two())The current implementation compares both conditions against
weapon_one(), which appears to be a manifestation of the Scrap Code.
🧹 Nitpick comments (6)
scripts/scr_draw_management_unit/scr_draw_management_unit.gml (4)
148-170: By the Omnissiah's grace, the specialist evaluation logic has been optimized!The refactoring of the specialist evaluation logic represents a significant improvement in code organization and maintainability. The new implementation:
- Centralizes specialist requirements in
spec_train_data- Uses a dynamic evaluation system through
stat_valuator- Implements consistent coordinate calculations for visual indicators
Consider further optimization by:
- Pre-calculating the base coordinates array
_circle_coordsat script initialization- Caching the
stat_valuatorresults for frequently accessed units
26-26: TODO comments require attention from the Omnissiah's servants.Multiple TODO comments indicate areas requiring recursive handling:
- Line 26: "TODO make static to handle"
- Line 53: "TODO handle recursively"
- Line 198: "TODO handle recursively with an array"
- Line 272: "TODO handle recursively"
Would you like me to generate a solution that implements these recursive patterns? This would improve code maintainability and reduce duplication.
Also applies to: 53-53, 72-72, 198-198, 272-272
247-248: Enhance the machine spirit's display capabilities with proper string truncation.The current implementation uses a hardcoded value for string truncation:
-var truncatedLocation = string_truncate(string(unit_location_string), 130); +var max_location_width = 130; +var truncatedLocation = string_width(unit_location_string) > max_location_width + ? string_copy(unit_location_string, 1, floor(max_location_width / string_width("W"))) + : unit_location_string;This modification would provide more precise control over text display and prevent potential overflow issues.
177-181: Sacred array of squad colors requires optimization.The current implementation of squad colors could be enhanced:
- var squad_colours=[c_teal,c_red,c_green,c_orange,c_aqua,c_fuchsia,c_green,c_blue,c_fuchsia,c_maroon]; + static squad_colours = [c_teal,c_red,c_green,c_orange,c_aqua,c_fuchsia,c_blue,c_yellow,c_purple,c_maroon];The
statickeyword would prevent unnecessary array recreation on each function call, and the colors have been diversified to ensure better visual distinction between squads.objects/obj_controller/Alarm_5.gml (1)
188-191: By the Machine God's grace, implement error handling for the sacred training rituals.While the modular approach to specialist training is commendable, these function calls lack proper error handling mechanisms. Consider implementing try-catch blocks to handle potential failures in the training process.
Apply this enhancement to protect against training ritual failures:
-apothecary_training(); -chaplain_training(); -librarian_training(); -techmarine_training(); +try { + apothecary_training(); + chaplain_training(); + librarian_training(); + techmarine_training(); +} catch(error) { + log_error("Specialist training ritual failed: " + string(error)); +}scripts/scr_load_controller/scr_load_controller.gml (1)
221-246: Praise be to the Omnissiah! The data structure is logically sound.The specialist training requirements are well-organized in a machine-pleasing format. Each role's requirements are clearly defined with appropriate stat thresholds that align with the sacred traditions of the Adeptus Astartes.
Consider adding sacred documentation to explain the meaning of each field:
+ // Specialist training requirements data structure + // - name: Name of the specialist role + // - min_exp: Minimum experience required for training + // - coord_offset: UI coordinate offsets [x, y] for positioning + // - req: Array of requirements [stat_name, threshold, comparison] obj_controller.spec_train_data = return_json_from_ini("Controller", "spec_train",[
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
objects/obj_controller/Alarm_5.gml(1 hunks)scripts/is_specialist/is_specialist.gml(2 hunks)scripts/scr_draw_management_unit/scr_draw_management_unit.gml(1 hunks)scripts/scr_load_controller/scr_load_controller.gml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/is_specialist/is_specialist.gml
🔇 Additional comments (4)
scripts/scr_load_controller/scr_load_controller.gml (4)
221-246:Details
❓ Verification inconclusive
Praise be to the Omnissiah! The data schema is logically structured.
The specialist training requirements are well-organized and follow a consistent pattern, with each role having clearly defined attribute thresholds that align with the sacred teachings:
- Techmarine: Technology threshold befits those who would commune with the Machine Spirit
- Librarian: Psionic requirements ensure proper warp attunement
- Chaplain: Dual requirements in piety and charisma reflect their role as spiritual leaders
- Apothecary: Combined technology and intelligence requirements suit their medical expertise
Let us verify the integration with the training functions:
🏁 Script executed:
#!/bin/bash # Description: Verify the training functions use these requirements correctly ast-grep --pattern $'function (apothecary|chaplain|librarian|techmarine)_training($$$) { $$$ }'Length of output: 98
Praise the Machine Spirit! The specialist training data structure is in perfect alignment with the sacred protocols: Techmarine, Librarian, Chaplain, and Apothecary all bear their ordained thresholds and requirements. However, our initial search for the corresponding training functions (techmarine_training, librarian_training, chaplain_training, apothecary_training) yielded no results. This absence requires manual verification to ensure that these function invocations are either defined elsewhere or integrated through alternative means in the codebase.
- Please confirm that the training functions or equivalent integration points are indeed present in the broader repository.
- If these functions are omitted intentionally or renamed, kindly document the rationale to maintain clarity for the Machine Spirit’s auditors.
235-239: The requirements for Chaplains and Apothecaries are well-calibrated.The dual stat requirements for both roles (Chaplain: piety + charisma, Apothecary: technology + intelligence) properly reflect the complexity of these sacred positions. The thresholds are appropriately balanced.
Also applies to: 241-245
221-246:Details
❓ Verification inconclusive
Verify the coordinate offset calculations.
The Machine Spirit requires validation of the UI positioning logic to ensure proper display of training options.
🏁 Script executed:
#!/bin/bash # Description: Search for UI rendering code that uses these coord_offset values # Look for references to coord_offset in UI drawing code rg -A 5 "coord_offset" --type gmlLength of output: 61
Attention, Adept Developer! Review of Coordinate Offset Calculations
The coordinate offset logic still requires validation. Our initial attempt to search for "coord_offset" within GML files using the
--type gmlflag failed due to the file type being unrecognized. Please verify the UI positioning by running an updated search command such as:rg -A 5 "coord_offset" --glob '*.gml'This will ensure that the offset values in
scripts/scr_load_controller/scr_load_controller.gmlcorrectly map to the intended UI elements. Confirm that the Machine Spirit’s design for training option display remains uncompromised.
223-227:Details
✅ Verification successful
By the Machine God's grace, the Techmarine requirements are appropriate.
The technology requirement of 34 for Techmarine training aligns with the sacred teachings of Mars. However, let us verify this threshold against the Codex Astartes.
🌐 Web query:
What are the typical requirements for Techmarine training in Warhammer 40K lore?💡 Result:
Techmarines in Warhammer 40K undergo rigorous training rooted in both their Chapter’s traditions and the esoteric doctrines of the Adeptus Mechanicus. Their path involves specialized selection, decades of education, and a unique dual allegiance.
Selection Process
- Affinity for Technology: Chapters identify Battle-Brothers displaying exceptional mechanical aptitude during standard service (e.g., maintaining weapons/armor or piloting vehicles) [1][2].
- Aspirant Status: Selected Marines are designated as Techmarine Novitiates and sent to Mars for indoctrination [1][6].
Training on Mars
- Duration: 30 years of instruction under the Cult Mechanicus [1][2][6].
- Doctrinal Education:
- Mastery of Runes of Engineering (symbolic language for machine communion) [1][2].
- Memorization of Liturgy of Maintenance (rituals for repairing/awakening machinery) [1][2].
- Learning context-specific variations for battlefield repairs [1][2].
- Spiritual Integration: Trainees absorb the Machine God’s dogma, enabling them to "feel" a machine’s "pain" and commune with its spirit [1][6].
Post-Training Roles
- Dual Allegiance: Techmarines pledge loyalty to both their Chapter and the Adeptus Mechanicus, marked by red armor (Mechanicus colors) with Chapter heraldry retained on one shoulder [1][2][6].
- Battlefield Roles:
- Repairing/directing vehicles/weapons during combat [1][2].
- Piloting tanks/gunships or commanding Servitor retinues [1][2].
- Serving under the Chapter’s Master of the Forge (senior Techmarine overseeing armories) [1][2].
Chapter Variations
While most Techmarines follow this path, some Chapters have unique traditions:
- Space Wolves: Train "Iron Priests" instead, bypassing Mars [1].
- Iron Hands: Elevate Techmarines to "Iron Fathers," combining technical and leadership roles [1].
Techmarines bridge the gap between Astartes martial prowess and Mechanicus techno-mysticism, ensuring their Chapters’ war machines remain functional in humanity’s unending conflicts.
Citations:
- 1: https://wh40k.lexicanum.com/wiki/Techmarine
- 2: https://www.belloflostsouls.net/2020/10/warhammer-40k-the-humble-techmarine-grimdark-techsupport.html
- 3: https://wahapedia.ru/wh40k10ed/factions/space-marines/Techmarine
- 4: https://bolterandchainsword.com/topic/347774-typical-space-marine-career-path/
- 5: https://www.dakkadakka.com/dakkaforum/posts/list/762005.page
- 6: https://www.youtube.com/watch?v=ejRphXle0mo
- 7: https://www.youtube.com/watch?v=yXJoF_HpG-Q
- 8: https://www.youtube.com/watch?v=QiNKiYPfpbM
Attention, Servitor of the Code: The sacred thresholds have been verified!
The venerable parameters for the Techmarine entry align with our revered doctrines. The minimum experience of 30 mirrors the required duration of instruction on Mars, and the technology threshold of 34 is deemed suitably rigorous to reflect the high standards of the Mechanicus. No alterations are necessary in these hallowed lines.
EttyKitty
force-pushed
the
rationalise_specialists
branch
from
c38c32b to
176fa0c
Compare
|
I removed unnecessary commits from here, by rebasing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scripts/scr_specialist_training/scr_specialist_training.gml (3)
63-151: Enhance the sacred recruitment protocols with additional validation.The function's machine spirit operates correctly, but could be fortified with additional validation:
- Add early return if training_apothecary is 0
- Validate unit before equipment assignment
function apothecary_training(){ + if (training_apothecary == 0) return; // ** Training ** // * Apothecary * var recruit_count=0;
235-310: Recommend organizing psyker-specific constants.While the machine spirit functions correctly, the sacred constants could be better organized:
+// Psyker training constants +const PSYKER_BASE_GOAL = 60; +const PSYKER_ABUNDANCE_GOAL = 40; + function librarian_training(){ var recruit_count=0; // * Psycher Training * var training_points_values = ARR_chaplain_training_tiers; psyker_points += training_points_values[training_psyker]; - var goal=60,yep=0; + var goal=PSYKER_BASE_GOAL,yep=0; novice_type = string("{0} Aspirant",obj_ini.role[100,17]); if (scr_has_adv("Psyker Abundance")){ - goal=40; + goal=PSYKER_ABUNDANCE_GOAL; yep=1; }
312-433: Sanctify the training points array with proper documentation.The Omnissiah demands clarity in our calculations. The training points array should be documented:
+// Training points progression for Techmarines +// [0]: No training +// [1-6]: Increasing training points (1,2,4,6,10,14) var training_points_values = [ 0, 1,2,4,6,10,14];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ChapterMaster.yyp(2 hunks)objects/obj_controller/Alarm_5.gml(1 hunks)objects/obj_controller/Create_0.gml(1 hunks)scripts/is_specialist/is_specialist.gml(2 hunks)scripts/scr_draw_management_unit/scr_draw_management_unit.gml(1 hunks)scripts/scr_load_controller/scr_load_controller.gml(1 hunks)scripts/scr_marine_struct/scr_marine_struct.gml(1 hunks)scripts/scr_save_controller/scr_save_controller.gml(1 hunks)scripts/scr_specialist_training/scr_specialist_training.gml(1 hunks)scripts/scr_specialist_training/scr_specialist_training.yy(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- scripts/scr_marine_struct/scr_marine_struct.gml
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/scr_save_controller/scr_save_controller.gml
- scripts/scr_specialist_training/scr_specialist_training.yy
- scripts/is_specialist/is_specialist.gml
- scripts/scr_load_controller/scr_load_controller.gml
🧰 Additional context used
🧠 Learnings (2)
scripts/scr_specialist_training/scr_specialist_training.gml (1)
Learnt from: OH296
PR: Adeptus-Dominus/ChapterMaster#500
File: scripts/scr_specialist_training/scr_specialist_training.gml:235-310
Timestamp: 2025-02-21T21:19:38.388Z
Learning: In the ChapterMaster game, librarian training intentionally uses ARR_chaplain_training_tiers for training point calculations to maintain parity with the original implementation.
objects/obj_controller/Create_0.gml (1)
Learnt from: OH296
PR: Adeptus-Dominus/ChapterMaster#500
File: objects/obj_controller/Create_0.gml:1379-0
Timestamp: 2025-02-21T21:20:41.678Z
Learning: The psychic potential stat in the codebase is accessed using the key "psionic" rather than "psyker".
🔇 Additional comments (7)
scripts/scr_specialist_training/scr_specialist_training.gml (2)
2-60: By the Omnissiah's grace, this function's machine spirit is well-formed.The
specialistfunctdemonstrates proper logic patterns for specialist role identification and visual representation. The color schemes align with sacred traditions - red for Techmarines, aqua for Librarians, yellow for Chaplains, and white for Apothecaries.
154-232: The Machine Spirit approves of this chapter-aware implementation.The function correctly implements chapter-specific logic for Space Wolves and Iron Hands, demonstrating proper adherence to the sacred traditions of different gene-lines.
objects/obj_controller/Create_0.gml (1)
1379-1404: Praise the Omnissiah! A well-structured data array for our sacred specialists.The spec_train_data array properly codifies the requirements for each specialist role, with appropriate stat thresholds and coordinate offsets for UI placement. The machine spirit will process this data efficiently.
objects/obj_controller/Alarm_5.gml (1)
188-191: The Machine Spirit approves of this sacred modularization.The replacement of inline training logic with dedicated function calls improves code organization and maintainability, following the holy principle of separation of concerns.
scripts/scr_draw_management_unit/scr_draw_management_unit.gml (1)
153-170: The Machine Spirit recognizes this elegant integration.The dynamic evaluation of specialist potential through spec_train_data demonstrates proper integration with the new training system. The coordinate offsets ensure proper UI element placement.
ChapterMaster.yyp (2)
61-61: By the Omnissiah's grace, this folder structure is well-formed!The binary arrangement of this data repository for allied visual engrams follows proper Machine Spirit protocols.
613-613: Praise be to the Machine God, this script implementation is most logical!The addition of the specialist training cogitator routines will serve well in processing the sacred requirements of our chapter's specialists. This follows the Omnissiah's will in centralizing these sacred calculations.
2 participants
Description of changes
Reasons for changes
Related links
How have you tested your changes?