feat: extract weapon stats to global variable / no impact on functionality#1080
feat: extract weapon stats to global variable / no impact on functionality#1080OH296 merged 3 commits intoAdeptus-Dominus:mainfrom
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughTech-Priest: repository ignores two sprite cache artefacts, an audio group's identifier was cleared in the project manifest, and a large, public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.gitignoreChapterMaster.yypscripts/scr_en_weapon/scr_en_weapon.gml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.gml
⚙️ CodeRabbit configuration file
**/*.gml: - Constants ofmacrotype MUST have a space between the constant name and its value. Without it, the compiler will throw an error.
- WRONG:
#macro COLOR_RED11119- RIGHT:
#macro COLOR_RED 11119- Color codes in the code SHOULDN'T have any spaces in their ID.
- WRONG:
# 80bf40- RIGHT:
#80bf40- All code should comply with the main GML documentation: https://manual.gamemaker.io/beta/en/GameMaker_Language/GML_Reference/GML_Reference.htm
Files:
scripts/scr_en_weapon/scr_en_weapon.gml
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - Do not prioritize the shortest code. Prioritize code that explicitly reveals its intent.
- Understandable variable naming is extremely important.
- If a solution is "clever" but hard to parse mentally, request a refactor to a more verbose but clearer approach.
- Apply the "Rule of Three": suggest abstraction when logic is repeated three times or more times.
- For subjective improvements (naming, architectural choice), do not give a concrete suggestion immediately. Instead, ask a guiding question to prompt the user's reflection.
- Every suggestion for a code change must include a brief justification.
- If a TODO comment is added, ask the user if you should create a GitHub issue for this TODO.
- If a TODO comment is deleted, remind the user if there is an active GitHub issue related to that comment.
Files:
scripts/scr_en_weapon/scr_en_weapon.gmlChapterMaster.yyp
🧠 Learnings (3)
📓 Common learnings
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.
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.
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.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.
Learnt from: EttyKitty
Repo: Adeptus-Dominus/ChapterMaster PR: 0
File: :0-0
Timestamp: 2025-04-05T20:58:21.881Z
Learning: In the ChapterMaster game, there's a need to refactor hardcoded item effects and move them to a "specials" property of items, following the pattern established with the `combi_tool` special. This improves code maintainability and makes effects more explicit in item definitions.
📚 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:
scripts/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:
scripts/scr_en_weapon/scr_en_weapon.gml
🔇 Additional comments (3)
.gitignore (1)
18-21: Sprite cache artefacts rightly consigned to the voidTech-Priest, ignoring
sprites.import.jsonandsprites.info.jsonis appropriate for machine-specific import and cache state, and the comments document the override path clearly. No further changes demanded by the machine spirit.ChapterMaster.yyp (1)
5-5: Verify audiogroup identity normalisation foraudiogroup_defaultTech-Priest, the
$GMAudioGroupfield foraudiogroup_defaultnow bears an empty string rather than a version tag. This looks like an IDE-generated normalisation, but it does alter the serialized identity of the audio group.Confirm this change was produced by your current GameMaker version and that other Magi are aligned on the same toolchain, lest divergent IDEs continually rewrite this field.
scripts/scr_en_weapon/scr_en_weapon.gml (1)
1192-1198: Canonical weapon nomenclature harbours silent failures waiting to manifestTech-Priest, the sanctified weapon designations revealed here possess cunning merit. Yet the archives contain lingering aberrations:
"Twin-Linked Heavy Bolters"(accursed hyphenation) versus"Twin Linked Heavy Bolters"coexist in the ancient case structures, whilst generic phantoms—"Claws","Autogun","Flamer"—persist where faction precision now reigns.When other systems begin venerating
global.en_weapons, such orthographic discrepancies shall birth silent lookup failures, cryptic failures of the darkest kind.Contemplate thy design path:
- Inscribe a normalisation mechanism (mapping corrupted spellings to sanctified keys), or
- Purify all weapon invocations across the codebase to emit only canonical identifiers
This architectural decision, deferred though it may be, must be crystallised ere the machine spirits grow restless with creeping entropy.
⛔ Skipped due to learnings
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.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.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.Learnt from: EttyKitty Repo: Adeptus-Dominus/ChapterMaster PR: 424 File: datafiles/data/psychic_powers.json:253-270 Timestamp: 2025-03-09T02:23:07.284Z Learning: The "Kamehameha" power name in the psychic powers system should be preserved as an artifact of the original codebase, despite it being a non-Warhammer 40k reference.Learnt from: MCPO-Spartan-117 Repo: Adeptus-Dominus/ChapterMaster PR: 526 File: objects/obj_popup/Draw_0.gml:234-239 Timestamp: 2025-03-01T11:06:25.427Z Learning: The comment "Need to modify ^^^^ based on if it is chaos or daemonic" in the artifact gifting code is intentionally kept as a reminder that this implementation is not yet finished, despite the significant refactoring already done.
cyotas
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Purpose and Description
Weapons stats have been extracted from switch cases into a global variable.
Special effects (poison weapons deal more damage against weakness to poison, siege weapons bonus range vs forts) have been converted into tags (Special: "Poison", "Siege") with comments describing what those effects actually did, to-be-unified later.