-
Notifications
You must be signed in to change notification settings - Fork 32
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: Trait cost adjustments #490
Conversation
Adjust prices and effects to be more uniform for "Enemy:" traits and specialist traits
📝 WalkthroughWalkthroughThe changes update gameplay parameters by modifying the properties of certain advantages and disadvantages along with adjustments to the unit initialization logic. Specific numeric values for attributes associated with enemy advantages and thematic disadvantages have been revised, and some metadata arrays have been updated. Additionally, the script responsible for custom initialization now alters the adjustments applied to unit counts such as techmarines, tenth, codiciery, chaplains, and apothecary based on these advantages and disadvantages. These modifications collectively recalibrate the character attributes and faction unit configurations without altering the overall structure of the system. 🪧 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 review |
✅ Actions performedReview triggered.
|
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: 0
🧹 Nitpick comments (4)
scripts/scr_initialize_custom/scr_initialize_custom.gml (4)
865-970
: Standardized trait adjustments improve balance.The trait adjustments have been standardized to consistently use +/-5 for specialist/tenth company trade-offs, making the system more balanced and predictable. This change helps ensure that the cost of chapter traits better reflects their actual value.
Consider documenting these standardized values in a configuration file or constants to make future balancing easier to maintain.
3292-3380
: Enhance helper function robustness.While the unit initialization helper is well-structured, it could benefit from:
- Input validation for critical parameters
- Extraction of Librarian power initialization into a separate function
- More explicit documentation of the default parameter behavior
Consider this refactor:
function add_unit_to_company(ttrpg_name, company, slot, role_name, role_id, wep1="default", wep2="default", gear="default", mobi="default", armour="default"){ + // Validate inputs + if (!is_string(ttrpg_name) || !is_real(company) || !is_real(slot) || !is_string(role_name) || !is_real(role_id)) { + throw("Invalid parameters provided to add_unit_to_company"); + } + // ... existing initialization code ... - if(role_id == eROLE.Librarian){ - var let = ""; - if (obj_creation.discipline = "default") { - let = "D"; - } - // ... more librarian initialization ... - } + if(role_id == eROLE.Librarian){ + initialize_librarian_powers(company, slot); + } } + +function initialize_librarian_powers(company, slot) { + var let = ""; + if (obj_creation.discipline = "default") { + let = "D"; + } + // ... rest of librarian initialization ... +}
3271-3286
: Improve vehicle initialization flexibility.The vehicle initialization function could be more robust and configurable:
- It lacks input validation
- Contains hardcoded values that could be configurable
- Is tightly coupled to the obj_ini object
Consider this enhancement:
+// Define vehicle constants +#macro DEFAULT_VEHICLE_HP 100 +#macro DEFAULT_VEHICLE_CHAOS 0 +#macro DEFAULT_VEHICLE_PILOTS 0 +#macro DEFAULT_VEHICLE_LID -1 +#macro DEFAULT_VEHICLE_WID 2 -function add_veh_to_company(name, company, slot, wep1, wep2, wep3, upgrade, accessory) { +function add_veh_to_company(name, company, slot, wep1, wep2, wep3, upgrade, accessory, config = {}) { + // Validate inputs + if (!is_string(name) || !is_real(company) || !is_real(slot)) { + throw("Invalid parameters provided to add_veh_to_company"); + } + obj_ini.veh_race[company, slot] = 1; obj_ini.veh_loc[company, slot] = obj_ini.home_name; obj_ini.veh_role[company, slot] = name; obj_ini.veh_wep1[company, slot] = wep1; obj_ini.veh_wep2[company, slot] = wep2; obj_ini.veh_wep3[company, slot] = wep3; obj_ini.veh_upgrade[company, slot] = upgrade; obj_ini.veh_acc[company, slot] = accessory; - obj_ini.veh_hp[company, slot] = 100; - obj_ini.veh_chaos[company, slot] = 0; - obj_ini.veh_pilots[company, slot] = 0; - obj_ini.veh_lid[company, slot] = -1; - obj_ini.veh_wid[company, slot] = 2; + obj_ini.veh_hp[company, slot] = config[? "hp"] ?? DEFAULT_VEHICLE_HP; + obj_ini.veh_chaos[company, slot] = config[? "chaos"] ?? DEFAULT_VEHICLE_CHAOS; + obj_ini.veh_pilots[company, slot] = config[? "pilots"] ?? DEFAULT_VEHICLE_PILOTS; + obj_ini.veh_lid[company, slot] = config[? "lid"] ?? DEFAULT_VEHICLE_LID; + obj_ini.veh_wid[company, slot] = config[? "wid"] ?? DEFAULT_VEHICLE_WID; }
3383-3402
: Enhance struct cloning robustness and performance.The DeepCloneStruct function could benefit from:
- Error handling for circular references
- Performance optimization for large structures
- Type validation
Consider this enhancement:
+/// @description Creates a deep copy of a struct or array, with circular reference detection +/// @param {Struct|Array} clone_struct The structure to clone +/// @returns {Struct|Array} A deep copy of the input function DeepCloneStruct(clone_struct) { + static seen = ds_map_create(); + + // Handle circular references + if (is_struct(clone_struct) || is_array(clone_struct)) { + var ref_id = string(ptr(clone_struct)); + if (ds_map_exists(seen, ref_id)) { + throw("Circular reference detected in DeepCloneStruct"); + } + ds_map_add(seen, ref_id, true); + } + if (is_array(clone_struct)) { var len = array_length(clone_struct); var arr = array_create(len); for (var i = 0; i < len; ++i) { arr[i] = DeepCloneStruct(clone_struct[i]); } + ds_map_delete(seen, string(ptr(clone_struct))); return arr; } else if (is_struct(clone_struct)) { var stc = {}; var nms = variable_struct_get_names(clone_struct); var len = array_length(nms); for (var i = 0; i < len; ++i) { var nm = nms[i]; stc[$nm] = DeepCloneStruct(clone_struct[$nm]); } + ds_map_delete(seen, string(ptr(clone_struct))); return stc; } + + // Validate primitive types + if (!is_real(clone_struct) && !is_string(clone_struct) && !is_bool(clone_struct) && clone_struct != undefined) { + throw("Unsupported type in DeepCloneStruct"); + } return clone_struct; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
objects/obj_creation/Create_0.gml
(4 hunks)scripts/scr_initialize_custom/scr_initialize_custom.gml
(3 hunks)
🔇 Additional comments (2)
objects/obj_creation/Create_0.gml (2)
665-665
: LGTM! Standardizing enemy trait costs improves balance.The standardization of all "Enemy:" trait values to 20 points creates a more balanced and intuitive system, as the combat bonus against different enemy types should have equivalent value.
Also applies to: 671-671, 677-677, 683-683
720-720
: LGTM! Improved trait metadata and cost adjustments.The changes improve the trait system by:
- Using more specific metadata tags (e.g., "Librarians" instead of "Specialists")
- Adjusting the "Reverent Guardians" cost to better reflect its value
Also applies to: 726-727, 732-732
@KRdaMystic, can't really comment on the balancing stuff and still don't understand what's the point of meta stuff. I'll check optional comments that coderabbit left and merge. |
metat stuff just stops you being able to take two contradictory things. e.g you can't have two things marked with the same meta tag like psyker intolerant and psyker abundance. Stops annoying checks down the line |
comments are ouside of scope of pr therefore will merge |
Description of changes
Reasons for changes
How have you tested your changes?