-
Couldn't load subscription status.
- Fork 39
fix: Daemonworld exterminatus disposition hit #668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Daemonworld exterminatus disposition hit #668
Conversation
This fix handles exterminatus for demon worlds also makes all disposition increase universal across factions
|
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 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe update modifies the 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
scripts/scr_destroy_planet/scr_destroy_planet.gml (1)
1-244: 🧹 Nitpick (assertive)Holistic evaluation of the code indicates appropriate modifications
The alterations to the logic matrices successfully implement the requirements specified in the sacred task document. The binary distinction between daemon worlds and temporary demonic incursions has been correctly established.
However, the function remains bloated and requires additional modularization as hinted by the TODO comment on line 96. I recommend refactoring this function in a future iteration to separate the penalty logic from the planet destruction mechanism.
Consider extracting the disposition and faction status changes into separate functions such as:
scr_apply_exterminatus_penalties(planet_type, owner, first)scr_update_faction_relations(planet_type, owner)This would enhance maintainability and readability, allowing future tech-priests to service this code with greater efficiency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/scr_destroy_planet/scr_destroy_planet.gml(4 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_destroy_planet/scr_destroy_planet.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_destroy_planet/scr_destroy_planet.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_destroy_planet/scr_destroy_planet.gml
🔇 Additional comments (2)
scripts/scr_destroy_planet/scr_destroy_planet.gml (2)
97-97: Additional condition guards against illogical penalty application!The addition of
(you.p_type[baid]!="Daemon")creates an appropriate exception in the logic engine. This prevents unwarranted diplomatic penalties when purging daemon worlds that have corrupted Mechanicus holdings. The Machine God approves of this logical optimization.
134-134: Logical safeguard implemented successfully!The machine spirits are pleased with this correction. Adding the daemon world exception prevents inappropriate disposition penalties when exterminating daemonic corruption from worlds associated with the Ecclesiarchy.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@MCPO-Spartan-117 that's alright i will do the modification |
1- Condition for demon worlds not demonic incursion which are temporary and not necessarily demon world hence the unintended behaviour.
2- Made disposition increase universal for all loyalist factions, before it was for inquisition and Ecclesiarchy which made no sense.
Purpose
Purge the heretics without headache.
Describe your changes/additions
Literally and (you.p_type[baid]!="Daemon")
obj_controller.disposition[2] += 5;
obj_controller.disposition[3] += 5;
What can/needs to be improved/changed
A lot Feel free to do so.
Testing done
They didn't shoot me in sight so far
Related things and/or additional context
yes #635 contains the fix but i forgot to make it standalone because i'm lazy also accident.