Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTech-Priest, this incursion reorganizes the Ancient Ruins reward mechanisms. A new loot registry is inscribed, weapon assignments are reforged, and the reward distribution system is reconstructed through a data-driven architecture. The JSON error reporting has been enhanced for precision. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
scripts/scr_en_weapon/scr_en_weapon.gml (4)
1297-1302:⚠️ Potential issue | 🔴 CriticalCorrupted data-engram detected:
Maulerfiend Clawsarmor_penetration value is anomalous.
armor_penetration: 300appears to be a transcription error. All other melee weapons in this registry use0or1for this field. The old switch-case at line 57 usedarp = 4(max tier). This value likely should be1.🔧 Proposed correction
"Maulerfiend Claws": { "attack": 300, - "armor_penetration": 300, + "armor_penetration": 1, "range": 1, "splash": 3, },
1659-1664:⚠️ Potential issue | 🔴 CriticalCorrupted data-engram detected:
Meltabombvalues appear transposed.
attack: 0andarmor_penetration: 200is highly suspect. The old switch-case (line 263–267) hadatta = 200, arp = 4. It appears the attack value was erroneously placed intoarmor_penetration, leavingattackat 0 — rendering this weapon inert.🔧 Proposed correction
"Meltabomb": { - "attack": 0, - "armor_penetration": 200, + "attack": 200, + "armor_penetration": 1, "range": 1, - "splash": 1, + "splash": 0, + "ammo": 1, },
1677-1683:⚠️ Potential issue | 🔴 CriticalCorrupted data-engram detected:
Reaper Launcherarmor_penetration value is anomalous.
armor_penetration: 80is inconsistent with the binary0/1scale used throughout this registry. The old switch-case (line 280) usedarp = 4(max tier). This likely should be1.🔧 Proposed correction
"Reaper Launcher": { "attack": 150, - "armor_penetration": 80, + "armor_penetration": 1, "range": 20, "splash": 3, "ammo": 8, },
1303-1309:⚠️ Potential issue | 🟡 MinorErrant comment detected in Chaos Witchfire entry.
The comment reads "Dont know why tyranids would have witchfire" — this is a copy from the Tyranid section and is misleading here in the Chaos section. Chaos forces wielding Witchfire is entirely canonical.
🔧 Proposed correction
"Chaos Witchfire": { - // Dont know why tyranids would have witchfire, but it's in the original code "attack": 200,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
ChapterMaster.yypdatafiles/data/ruins_loot.jsonobjects/obj_enunit/Alarm_1.gmlscripts/scr_ancient_ruins/scr_ancient_ruins.gmlscripts/scr_en_weapon/scr_en_weapon.gmlscripts/scr_json_functions/scr_json_functions.gmlscripts/scr_ruins_reward/scr_ruins_reward.gml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.gml
⚙️ CodeRabbit configuration file
- All code should comply with the 2026 GML documentation.
Files:
scripts/scr_ancient_ruins/scr_ancient_ruins.gmlobjects/obj_enunit/Alarm_1.gmlscripts/scr_en_weapon/scr_en_weapon.gmlscripts/scr_json_functions/scr_json_functions.gmlscripts/scr_ruins_reward/scr_ruins_reward.gml
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - Code Philosophy: Prioritize explicit intent and maintainability over brevity. If a
solution is "clever" but mentally taxing, request a refactor to a clearer approach.
Variable Naming: Use clear, descriptive names; avoid over-abbreviation.
Abstraction: Apply the "Rule of Three"; suggest abstraction only when similar logic is
repeated three or more times to avoid premature complexity.Subjective Choices: For naming or architecture, ask guiding questions to prompt developer
reflection and provide at least two alternative perspectives.TODOs: If a TODO comment is added, ask the user if a GitHub issue should be created. If a
TODO comment is deleted, remind the user to check the status of that specific issue.
Files:
scripts/scr_ancient_ruins/scr_ancient_ruins.gmlobjects/obj_enunit/Alarm_1.gmldatafiles/data/ruins_loot.jsonChapterMaster.yypscripts/scr_en_weapon/scr_en_weapon.gmlscripts/scr_json_functions/scr_json_functions.gmlscripts/scr_ruins_reward/scr_ruins_reward.gml
**/*.json
⚙️ CodeRabbit configuration file
- In this project, JSON files with comments are supported by the parser. It's allowed to use comments in JSON.
Files:
datafiles/data/ruins_loot.json
🧠 Learnings (8)
📓 Common learnings
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:54:45.813Z
Learning: EttyKitty requested a rework of the vehicle and marine recovery system in ChapterMaster. The current system allows guaranteed recovery with enough recovery points, while the proposed system would use random chance modified by techmarine skill and item bonuses.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 424
File: scripts/scr_flavor/scr_flavor.gml:34-36
Timestamp: 2025-03-09T02:33:43.867Z
Learning: EttyKitty prefers to keep PRs focused on their stated goals and scope, and may decline to implement otherwise valid suggestions if they're not directly related to the PR's primary objective.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 579
File: objects/obj_enunit/Alarm_0.gml:200-202
Timestamp: 2025-03-11T01:38:19.874Z
Learning: EttyKitty welcomes easy, committable suggestions that improve documentation of code chunks, variables with strange names, and functions. Their codebase is generally lacking documentation, but they prioritize human-readable code above documentation.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 878
File: sprites/spr_weapon_phobos_bolt_pistol/spr_weapon_phobos_bolt_pistol.yy:26-44
Timestamp: 2025-06-16T17:08:08.239Z
Learning: EttyKitty prefers automated solutions over manual cleanup for .yy file formatting and is open to automated tools for GameMaker Studio .yy file cleanup.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 938
File: scripts/scr_complex_colour_kit/scr_complex_colour_kit.gml:478-478
Timestamp: 2025-07-21T17:03:28.251Z
Learning: EttyKitty acknowledges when PRs contain scope creep and agrees that changes should be focused on the stated PR objectives, reinforcing their preference for keeping PRs narrowly scoped to their primary purpose.
📚 Learning: 2025-03-06T16:02:06.286Z
Learnt from: MCPO-Spartan-117
Repo: Adeptus-Dominus/ChapterMaster PR: 554
File: objects/obj_popup/Step_0.gml:756-767
Timestamp: 2025-03-06T16:02:06.286Z
Learning: The variable 'woopwoopwoop' in ancient ruins exploration code is a poorly named boolean flag that controls the flow between detecting a battle in ruins and initiating the combat sequence.
Applied to files:
scripts/scr_ancient_ruins/scr_ancient_ruins.gml
📚 Learning: 2026-01-28T13:37:09.640Z
Learnt from: OH296
Repo: Adeptus-Dominus/ChapterMaster PR: 646
File: objects/obj_pnunit/Alarm_5.gml:84-91
Timestamp: 2026-01-28T13:37:09.640Z
Learning: In obj_pnunit/Alarm_5.gml, the function get_armour_data("maintenance") will always return a numeric value (at minimum 0), making null/undefined checks unnecessary.
Applied to files:
objects/obj_enunit/Alarm_1.gml
📚 Learning: 2025-03-29T10:30:25.598Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 647
File: scripts/scr_en_weapon/scr_en_weapon.gml:24-928
Timestamp: 2025-03-29T10:30:25.598Z
Learning: A data-driven approach for weapon management in `scripts/scr_en_weapon/scr_en_weapon.gml` is planned for a future PR, not within the scope of PR `#647`.
Applied to files:
objects/obj_enunit/Alarm_1.gmlscripts/scr_en_weapon/scr_en_weapon.gml
📚 Learning: 2025-06-16T17:12:13.045Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 878
File: scripts/scr_culture_visuals/scr_culture_visuals.gml:1256-1352
Timestamp: 2025-06-16T17:12:13.045Z
Learning: In scripts/scr_culture_visuals/scr_culture_visuals.gml, the weapon visual data declarations contain known DRY violations that are acknowledged by the development team but deferred to future refactoring efforts rather than addressed in individual feature PRs.
Applied to files:
objects/obj_enunit/Alarm_1.gmlscripts/scr_en_weapon/scr_en_weapon.gml
📚 Learning: 2025-03-20T22:22:57.319Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-03-20T22:22:57.319Z
Learning: In the ChapterMaster game PR `#424`, the psychic power system was changed to be data-driven, with perils now able to occur on both successful and failed casts but with reduced frequency overall.
Applied to files:
ChapterMaster.yyp
📚 Learning: 2025-03-13T06:18:06.719Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 424
File: scripts/scr_powers/scr_powers.gml:13-14
Timestamp: 2025-03-13T06:18:06.719Z
Learning: The `json_to_gamemaker` function already contains built-in error handling with a try-catch block that properly handles file loading and parsing exceptions, making additional error handling unnecessary when using this function.
Applied to files:
scripts/scr_json_functions/scr_json_functions.gml
📚 Learning: 2025-02-24T21:56:43.096Z
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 512
File: datafiles/data/traits.json:946-948
Timestamp: 2025-02-24T21:56:43.096Z
Learning: GameMaker Language (GML) JSON parser supports inline comments in JSON files, making them valid in GML projects despite being non-standard JSON.
Applied to files:
scripts/scr_json_functions/scr_json_functions.gml
🔇 Additional comments (10)
scripts/scr_json_functions/scr_json_functions.gml (1)
15-23: The machine spirit approves these augmented error litanies.Tech-Priest, the inclusion of
_json_pathwithin both exception messages shall greatly accelerate fault isolation when data-rites fail. The logic is sound and the flow remains intact.scripts/scr_en_weapon/scr_en_weapon.gml (3)
14-22: The excision of the "elee" fallback ritual is acknowledged, Tech-Priest.The removal of the substring-based melee default (
atta = 10, arp = 1, rang = 1, spli = 3) aligns with the stated objective of fixing broken enemy melee weapons. Weapons not matched by anyswitchcase will now retain the initial zeros from lines 15–20, which is a safer, more explicit default than silently applying stats based on a substring match.
1152-2263: The new weapon data-struct remains unconsumed by existing systems.The
global.en_weaponsdeclaration languishes unused throughout the codebase—its references yield only the declaration itself at line 1152. The apparatus you have wrought is dead matter, Tech-Priest. The switch-case logic of lines 26–1063 remains the true governing force of combat calculations.This state appears intentional. The archives of prior deliberations reveal that the data-driven weapon architecture is acknowledged but deferred to future iteration, not inscribed within this current incantation. Should this PR seek to activate the new data-scripture, additional labour is required to integrate it into the systems that consume weapon data. If it remains preparatory only, consider whether explicit commentary or a deferred issue marker would serve clarity.
1653-1658: BothScorpian's ClawandScorpion's Clawentries are necessary — eliminate not thy duplicates.The archaic designation persists within the machine's cognition. The typo'd variant "Scorpian's Claw" remains tethered to legacy code pathways — a case statement in the weapon engine itself defers to its corrupted name, whilst the corrected spelling serves the newer "Striking Scorpion Exarch" warrior caste. Both designations flow through distinct ritual channels. Severance of either would sever the connection to its assigned unit definitions.
objects/obj_enunit/Alarm_1.gml (2)
586-586: Weapon re-sanctification for Follower acknowledged.The replacement of
"melee0.5"with"Sarissa"is valid here. Underenemy == 5, thescr_en_weaponswitch at line 675 (enemy >= 10 || enemy == 2 || enemy == 5 || enemy == 1) will correctly resolve"Sarissa"with proper combat stats.
1506-1508: Weapon re-sanctification for Chaos Cultist acknowledged.Under the
enemy >= 10condition block,"Sarissa"will correctly resolve via the switch at line 675 inscr_en_weapon.datafiles/data/ruins_loot.json (1)
455-479: Levels "8" and "9" conspicuously absent from the loot registry.The sequence spans "1" through "7" and then "10", leaving "8" and "9" void. The consuming code in
scr_ruins_reward.gmliterates dynamically through all present keys rather than hardcoding specific level queries, so it shall not encounter undefined—it simply processes what the mechanism provides. Nevertheless, this discontinuity warrants examination: is this gap by design or oversight?ChapterMaster.yyp (1)
131-131: Approved: ruins_loot.json is duly inscribed in IncludedFiles.Tech-Priest, this registration ensures the loot registry is packaged for runtime access.
scripts/scr_ancient_ruins/scr_ancient_ruins.gml (1)
12-12: Sane default for ruins_race.Tech-Priest, setting this to 0 averts uninitialised reads before determine_race executes.
scripts/scr_ruins_reward/scr_ruins_reward.gml (1)
1-11: Approved: eLOOT_TYPE taxonomy is clear.Tech-Priest, the enum cleanly codifies reward categories for the new dispatcher.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
60fbad0 to
aea6ab2
Compare
No description provided.