feat: Planets affect recruit corruption and it's gaussian random#477
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new recruitment corruption logic that leverages a Gaussian distribution for calculating the corruption value. The changes refactor how corruption bonuses are handled: they are now determined independently from trials by combining a planet-specific bonus and a trial-specific bonus, both defined in [base, sd] format. The new find_recruit_corruption() function computes the corruption value by sampling from a Gaussian function and then aggregates these results to produce a final, positive corruption amount. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @EttyKitty - I've reviewed your changes - here's some feedback:
Overall Comments:
- In function find_recruit_corruption, in the trial bonus block, the check using _planet_type_data seems misplaced; change it to check _trial_type for a corruption_bonus to avoid unintended behavior.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
Blogaugis
left a comment
There was a problem hiding this comment.
I am a bit concerned that SD for gaussian distribution of 2 is a bit much, if my fiddling around with these values in the SM stats doc (especially if it comes to numbers low as that) is to go by. I think value of 1 might be better.
But otherwise, I guess it is alright.
|
@EttyKitty Let us know on suggestions and if you deem this ready to merge? |
Co-authored-by: Nelsonh <81228864+OH296@users.noreply.github.com>
It's okay to merge, if it looks okay from the side. |
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe changes introduce a new function that computes a recruitment trial corruption value based on the planet type. This function retrieves planet-specific corruption bonuses using a static data structure and applies a Gaussian distribution to calculate the corruption, ensuring the final value is non-negative. The existing recruitment logic now calls this function instead of using a fixed corruption value. Additionally, the trial data structure has been updated by replacing the previous corruption structure with a new array that holds the base corruption and its standard deviation. This modification affects multiple trial types, standardizing the way corruption values are represented and calculated across different recruitment scenarios. 🪧 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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scripts/scr_recruit_data/scr_recruit_data.gml (3)
64-122: Add input validation and documentation.The function implementation looks good but could benefit from:
- Input parameter validation
- Documentation explaining the purpose, parameters, and return value
Apply this diff to add validation and documentation:
+/// Calculates the corruption value for a recruit based on planet type and trial type. +/// @param {string} planet_type - The type of planet (e.g., "Hive", "Temperate", etc.) +/// @returns {number} The calculated corruption value, minimum 0 function find_recruit_corruption(planet_type){ + if (!is_string(planet_type)) { + show_error("planet_type must be a string", true); + } var _recruit_corruption = 0;
67-99: Consider differentiating planet type corruption values.Several planet types (Forge, Desert, Ice, Agri, Death, Lava) share identical corruption bonus values [5, 2]. Consider differentiating these based on their unique characteristics to add more variety to the gameplay.
493-502: Clean up commented code or add TODO.The commented code block suggests a potential feature for planet-specific corruption bonuses in trials. Either:
- Remove the commented code if the feature is not planned
- Add a TODO comment explaining the future feature plan
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/scr_recruit_data/scr_recruit_data.gml(8 hunks)
🔇 Additional comments (3)
scripts/scr_recruit_data/scr_recruit_data.gml (3)
144-144: Gate debug message in production code.The debug message should be gated to prevent it from appearing in production.
251-374: Review standard deviation values for balance.The standard deviation values are consistently set to:
- 2 for planet types
- 1 for trials
Consider if these values provide enough variability for an engaging gameplay experience. You might want to adjust these based on testing results.
64-502: Implementation aligns well with PR objectives!The changes successfully:
- Implement Gaussian distribution for corruption calculation
- Separate planet type corruption from trial corruption
- Update the data structures and UI to support the new system
Description of changes
find_recruit_corruption(planet_type)function, similar tofind_recruit_success_chance(). This thing returns the corruption value of a recruit, based on the selected trial and their source planet type, passed through a Gaussian distribution.[base, sd]format, for Gaussian distribution infind_recruit_corruption().Reasons for changes
Notes
Related links
How have you tested your changes?