First iteration of stat scaling system for weapons #253
Conversation
WalkthroughThe pull request updates the combat mechanics and equipment initialization, nyan. The Changes
Sequence Diagram(s)sequenceDiagram
participant M as Marine
participant A as Attack Function
participant GS as get_weapon_stat_bonus
M->>A: Call melee_attack / ranged_attack
A->>GS: Calculate weapon stat bonus
GS-->>A: Return bonus value
A-->>M: Return modified attack result
sequenceDiagram
participant S as System
participant ES as equipment_struct
S->>ES: Provide item_data, core_type, quality
ES-->>S: Return initialized equipment item
Poem
🪧 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
scripts/scr_weapon/scr_weapon.gml (1)
2553-2594: 🛠️ Refactor suggestionLet's clean up the commented code, nyan!
The commented-out code contains valuable logic for weapon modifications and vehicle upgrades, but keeping it as comments isn't ideal, nyan! Consider either:
- Implementing the features properly if they're needed
- Removing the comments if they're no longer relevant
- Moving the logic to a separate design document if it's for future reference
If you want to keep this logic, I can help create a new issue to track the implementation of these features, nyan! Would you like me to do that?
🧹 Nitpick comments (3)
scripts/scr_marine_struct/scr_marine_struct.gml (2)
1250-1252: Consider caching weapon stat bonuses for better performance, nyan!The code calculates stat bonuses for both weapons separately. We could optimize this by storing the results in variables, nyan!
-ranged_att += get_weapon_stat_bonus(_wep1) + get_weapon_stat_bonus(_wep2); +var wep1_bonus = get_weapon_stat_bonus(_wep1); +var wep2_bonus = get_weapon_stat_bonus(_wep2); +ranged_att += wep1_bonus + wep2_bonus;
1413-1417: Consider keeping detailed explanations for debugging purposes, nyan!The explanation strings have been simplified, which might make debugging harder, nyan! Consider keeping the detailed breakdowns but making them conditional based on a debug flag, nyan!
-var explanation_string = "1"; +var explanation_string = ""; +if (global.debug_mode) { + explanation_string += " Base: +10%#"; + explanation_string += string_concat(" WSxSTR: ", format_number_with_sign(round((((weapon_skill/100)*(strength/20))-1)*100)), "%#"); + explanation_string += string_concat(" EXP: ", format_number_with_sign(round((experience()/1000)*100)), "%#"); +} else { + explanation_string = "1"; +}scripts/scr_weapon/scr_weapon.gml (1)
2530-2552: Let's make the equipment constructor even better, nyan!The constructor is well-structured, but here are some suggestions to enhance it further:
- Consider using a 2D array to combine
namesanddefaultsfor better maintainability, nyan!- The
qualityparameter's default value could be more explicit as"standard"instead of"none", nyan!- The special handling of
stat_scalingcould be moved to a separate function for better clarity, nyan!Here's how you could refactor it:
-function equipment_struct(item_data, core_type,quality="none") constructor{ +function equipment_struct(item_data, core_type, quality="standard") constructor{ //This could be done with 2d arrays [[],[]] - var names = ["hp_mod", "description","damage_resistance_mod", "ranged_mod", "melee_mod","armour_value" ,"attack","melee_hands","ranged_hands","ammo","range","spli","arp","special_description", "special_properties", "abbreviation","tags","name","second_profiles","req_exp", "stat_scaling"]; - var defaults = [0,"",0,0,0,0,0,0,0,0,0,0,0,"",[],"",[],"",[],0, 0]; + var properties = [ + ["hp_mod", 0], + ["description", ""], + ["damage_resistance_mod", 0], + // ... rest of the properties + ["stat_scaling", 0] + ]; type = core_type; - for (var i=0;i<array_length(names);i++){ - if (struct_exists(item_data,names[i])){ - self[$names[i]] = item_data[$names[i]]; - if (quality!="none" && names[i] != "stat_scaling"){ - if (is_struct(self[$names[i]])){ - if (struct_exists(self[$names[i]],quality)){ - self[$names[i]]=self[$names[i]][$quality]; - } else { - self[$names[i]]=self[$names[i]].standard; - } + for (var i = 0; i < array_length(properties); i++) { + var prop_name = properties[i][0]; + var default_value = properties[i][1]; + + if (struct_exists(item_data, prop_name)) { + self[$prop_name] = item_data[$prop_name]; + if (prop_name != "stat_scaling") { + self[$prop_name] = _apply_quality_modifier(self[$prop_name], quality); } } else { - self[$names[i]]=defaults[i]; + self[$prop_name] = default_value; } } - variable_struct_set(self, "quality", quality=="none"?"standard":quality); + self.quality = quality; +} + +/// @function _apply_quality_modifier(value, quality) +function _apply_quality_modifier(value, quality) { + if (!is_struct(value)) return value; + return struct_exists(value, quality) ? value[$quality] : value.standard; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/scr_marine_struct/scr_marine_struct.gml(4 hunks)scripts/scr_weapon/scr_weapon.gml(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Format Code
scripts/scr_weapon/scr_weapon.gml
[error] 1-1: Unable to process file command 'env' successfully.
scripts/scr_marine_struct/scr_marine_struct.gml
[error] 1-1: Unable to process file command 'env' successfully.
🔇 Additional comments (1)
scripts/scr_weapon/scr_weapon.gml (1)
87-90: Nice addition of stat scaling to the Chainsword, nyan!The scaling values look well-balanced, with dexterity having a higher multiplier (1.4) than strength (1.2), which makes sense for a Chainsword's swift and precise attacks, nyan!
| static get_weapon_stat_bonus = function(_weapon) { | ||
| var _total_bonus = 0; | ||
| if (_weapon.stat_scaling != 0) { | ||
| var _stats = struct_get_names(_weapon.stat_scaling); | ||
| for (var i = 0; i < array_length(stats); i++) { | ||
| var _stat_name = stats[i]; | ||
| var _stat_value = variable_struct_get(self, stat_name); | ||
| if (stat_value != 0) { | ||
| _total_bonus += variable_struct_get(_weapon.stat_scaling, stat_name) * stat_value; | ||
| } | ||
| } | ||
| } | ||
| return _total_bonus; | ||
| } |
There was a problem hiding this comment.
Fix variable name inconsistencies to prevent runtime errors, nyan!
There are mismatches in variable names that would cause runtime errors, nyan! The stats array is used but never defined, and stat_name/stat_value are used incorrectly, nyan!
static get_weapon_stat_bonus = function(_weapon) {
var _total_bonus = 0;
if (_weapon.stat_scaling != 0) {
- var _stats = struct_get_names(_weapon.stat_scaling);
- for (var i = 0; i < array_length(stats); i++) {
- var _stat_name = stats[i];
- var _stat_value = variable_struct_get(self, stat_name);
- if (stat_value != 0) {
- _total_bonus += variable_struct_get(_weapon.stat_scaling, stat_name) * stat_value;
+ var _stats = struct_get_names(_weapon.stat_scaling);
+ for (var i = 0; i < array_length(_stats); i++) {
+ var _stat_name = _stats[i];
+ var _stat_value = variable_struct_get(self, _stat_name);
+ if (_stat_value != 0) {
+ _total_bonus += variable_struct_get(_weapon.stat_scaling, _stat_name) * _stat_value;
}
}
}
return _total_bonus;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static get_weapon_stat_bonus = function(_weapon) { | |
| var _total_bonus = 0; | |
| if (_weapon.stat_scaling != 0) { | |
| var _stats = struct_get_names(_weapon.stat_scaling); | |
| for (var i = 0; i < array_length(stats); i++) { | |
| var _stat_name = stats[i]; | |
| var _stat_value = variable_struct_get(self, stat_name); | |
| if (stat_value != 0) { | |
| _total_bonus += variable_struct_get(_weapon.stat_scaling, stat_name) * stat_value; | |
| } | |
| } | |
| } | |
| return _total_bonus; | |
| } | |
| static get_weapon_stat_bonus = function(_weapon) { | |
| var _total_bonus = 0; | |
| if (_weapon.stat_scaling != 0) { | |
| var _stats = struct_get_names(_weapon.stat_scaling); | |
| for (var i = 0; i < array_length(_stats); i++) { | |
| var _stat_name = _stats[i]; | |
| var _stat_value = variable_struct_get(self, _stat_name); | |
| if (_stat_value != 0) { | |
| _total_bonus += variable_struct_get(_weapon.stat_scaling, _stat_name) * _stat_value; | |
| } | |
| } | |
| } | |
| return _total_bonus; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/gobo_format.yml (1)
1-1:⚠️ Potential issuePipeline configuration alert: File extension mismatch, nyan!
The CI pipeline error indicates that.github/workflows/gobo_format.ymlis not a .gml file. Since this file is intended to be a GitHub Actions workflow configuration, please update the pipeline settings to exclude non-.gml files from formatting checks, nyan.🧰 Tools
🪛 GitHub Actions: Format Code
[error] 1-1: .github/workflows/gobo_format.yml is not a *.gml file.
🧹 Nitpick comments (2)
.github/workflows/gobo_format.yml (2)
43-44: Consider edge case handling for filenames in the changed files list, nyan!
The revised code uses thepastecommand to join the list of changed files into a space‐separated string, which works well if none of the filenames contain spaces. If there’s any chance that filenames might include spaces, consider using a delimiter that is less likely to appear (like a newline or a null character) and adjust the splitting logic accordingly, nyan.
46-46: Remove trailing whitespace to satisfy YAMLlint, nyan!
There are trailing spaces detected on line 46 which might lead to linting issues. Please remove them to ensure a clean YAML file, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gobo_format.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gobo_format.yml
[error] 46-46: trailing spaces
(trailing-spaces)
🪛 GitHub Actions: Format Code
.github/workflows/gobo_format.yml
[error] 1-1: .github/workflows/gobo_format.yml is not a *.gml file.
🔇 Additional comments (1)
.github/workflows/gobo_format.yml (1)
53-57: Great job splitting the changed files string into an array, nyan!
The use ofIFS=' 'along with thereadcommand to convert the space‐separated list into an array is clear and concise, making it easier to iterate over each file during formatting, nyan.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/gobo_format.yml (2)
53-57: Splitting Changed Files into an Array
UsingIFS=' ' read -r -a files <<< "$changed_files"works well in converting the space-separated list into an array, nyan! However, if there's any possibility that a file name might contain spaces (even though it's unlikely for.gmlfiles), you might want to consider a more robust delimiter, nyan.
46-46: Remove Trailing Whitespace
A tiny nitpick: there seems to be trailing whitespace at line 46, nyan! Removing it will help keep the YAML file neat and satisfy linting tools, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gobo_format.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gobo_format.yml
[error] 46-46: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/gobo_format.yml (2)
43-44: Improved Changed Files Filtering
The new command pipeline to list only.gmlfiles usinggrep -E '.*\.gml$'and then collapsing them into a space-separated string withpaste -s -d' 'is clear and efficient, nyan! Just make sure to monitor for any edge cases with unexpected file names, nyan.
71-77: Enhanced Formatting Error Feedback
The block checking for formatting issues withgit diff --quiet --exit-codeis purr-fect for capturing issues, nyan! The detailed error message that lists the files failing formatting will really help with debugging, nyan.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.github/workflows/gobo_format.yml (6)
43-45: Extraction of Changed .gml Files is Clear, nyan!
The command correctly filters for files ending with.gmland aggregates them into a space-separated string usingpaste, nyan. One potential improvement is to consider handling file names with spaces in the future, nyan.
46-46: Trim Trailing Spaces at Line 46, nyan!
A trailing space is detected at the end of line 46, which should be removed to comply with YAML lint guidelines, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
52-60: Loop Over Changed Files is Implemented Nicely, nyan!
The for-loop correctly splits the space-separatedchanged_filesstring into an array and iterates through each file to run the formatter, nyan. Just be mindful of potential issues with filenames that may contain spaces, nyan, although they are unlikely, nyan.
70-83: Formatting Check Block Works Well, nyan!
The block checking for formatting changes viagit diff --quiet --exit-codeis well-structured. It provides a clear error message along with the list of problematic files and exits with an error when needed, nyan. For consistency, consider ensuring no trailing spaces exist in the echoed output, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
75-75: Remove Trailing Spaces at Line 75, nyan!
Static analysis reports trailing spaces on line 75; please remove them for clean formatting according to YAML lint guidelines, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 75-75: trailing spaces
(trailing-spaces)
79-79: Remove Trailing Spaces at Line 79, nyan!
A trailing space is detected at line 79. Please trim it to maintain consistency with YAML formatting best practices, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 79-79: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gobo_format.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gobo_format.yml
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 79-79: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/gobo_format.yml (2)
43-44: Changed Files Extraction Logic is Neat, nyan!The new approach to extract
.gmlfiles usinggit diff,grep, andpasteis straightforward and works well for our formatting purposes, nyan! However, please double-check that filenames containing spaces are handled correctly or that such cases are unlikely, nyan.
46-46: Minor Nitpick: Remove Trailing Spaces, nyan!YAML lint has detected trailing spaces on line 46. Please remove the extra whitespace to maintain a clean and compliant file, nyan.
- echo "changed_files=$changed_files" >> $GITHUB_ENV + echo "changed_files=$changed_files" >> $GITHUB_ENV🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gobo_format.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gobo_format.yml
[error] 46-46: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/gobo_format.yml (2)
53-57: Array Splitting for Changed Files is Purr-fect, nyan!The splitting of the changed files string into an array and iterating over it using a for loop is clean and effective, nyan! Great job ensuring each file gets formatted individually, nyan.
71-78: Enhanced Formatting Error Handling is Adorable, nyan!The updated error handling block that checks for formatting discrepancies and clearly lists the problematic files is a very nice improvement to the workflow, nyan! This will make troubleshooting much easier, nyan.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/gobo_format.yml (4)
43-44: Extraction Command Robustness Improvement: The command for extracting.gmlfiles is concise and works fine for standard cases, nyan. However, filenames with spaces might not be properly handled by usingpaste -s -d' ', nyan. Consider using newline separation (for example, piping throughtr '\n' ' 'or usingreadarrayto directly capture newline-separated entries) for improved robustness, nyan.
53-55: Array Splitting Enhancement: UsingIFS=' 'to split the space-separated string into an array is functional for simple filenames, nyan. However, if any filename contains spaces, this might lead to issues, nyan. You might consider usingreadarray -t files <<< "$changed_files"to split on newlines instead, nyan.
57-57: For Loop and Formatter Invocation Check: The for-loop that iterates over each file is simple and clear, nyan. It could be beneficial to include error handling in case the./goboformatter command fails for a specific file, nyan, which would assist in pinpointing issues faster, nyan.
46-46: Remove Trailing Whitespace: There is trailing whitespace on this line, nyan. Please remove it to comply with YAML linting standards, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gobo_format.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gobo_format.yml
[error] 46-46: trailing spaces
(trailing-spaces)
.github/workflows/gobo_format.yml
Outdated
| echo "Checking for formatting changes..." | ||
| git diff --exit-code || exit 1 | ||
| if [ $? -ne 0 ]; then | ||
| failed_files=$(cat failed_files.txt) | ||
| if [ -z "$failed_files" ]; then | ||
| echo "No specific failed files identified." | ||
| else | ||
| echo -e "\n⚠️ Formatting failed for the following files:\n" | ||
| echo -e "```\n$failed_files\n```" | ||
| fi | ||
| exit 1 | ||
| else | ||
| echo "🎉 All formatting checks passed successfully!" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Revise Formatting Change Check Logic: The error handling block here has a logical flaw, nyan. The use of git diff --exit-code || exit 1 causes an immediate exit if formatting issues are found, making the subsequent block that prints out detailed information (like listing failed_files) unreachable, nyan. I recommend removing the || exit 1 and restructuring the conditional to properly capture the non-zero exit status from git diff, nyan. For example:
- echo "Checking for formatting changes..."
- git diff --exit-code || exit 1
- if [ $? -ne 0 ]; then
- failed_files=$(cat failed_files.txt)
- if [ -z "$failed_files" ]; then
- echo "No specific failed files identified."
- else
- echo -e "\n⚠️ Formatting failed for the following files:\n"
- echo -e "```\n$failed_files\n```"
- fi
- exit 1
- else
- echo "🎉 All formatting checks passed successfully!"
- fi
+ echo "Checking for formatting changes..."
+ if ! git diff --exit-code; then
+ failed_files=$(cat failed_files.txt)
+ if [ -z "$failed_files" ]; then
+ echo "No specific failed files identified."
+ else
+ echo -e "\n⚠️ Formatting failed for the following files:\n"
+ echo -e "```\n$failed_files\n```"
+ fi
+ exit 1
+ else
+ echo "🎉 All formatting checks passed successfully!"
+ fiThis change will ensure that detailed error messages are provided when formatting issues occur, nyan.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "Checking for formatting changes..." | |
| git diff --exit-code || exit 1 | |
| if [ $? -ne 0 ]; then | |
| failed_files=$(cat failed_files.txt) | |
| if [ -z "$failed_files" ]; then | |
| echo "No specific failed files identified." | |
| else | |
| echo -e "\n⚠️ Formatting failed for the following files:\n" | |
| echo -e "```\n$failed_files\n```" | |
| fi | |
| exit 1 | |
| else | |
| echo "🎉 All formatting checks passed successfully!" | |
| fi | |
| echo "Checking for formatting changes..." | |
| if ! git diff --exit-code; then | |
| failed_files=$(cat failed_files.txt) | |
| if [ -z "$failed_files" ]; then | |
| echo "No specific failed files identified." | |
| else | |
| echo -e "\n⚠️ Formatting failed for the following files:\n" | |
| echo -e " |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/gobo_format.yml (1)
46-46: Trailing spaces need to vanish, nyan!
Please remove the trailing spaces on this line to comply with YAML lint guidelines and keep the file tidy, nyan.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gobo_format.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gobo_format.yml
[error] 46-46: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/gobo_format.yml (2)
43-44: Efficient extraction of changed .gml files, nyan!
The use ofgit diffpiped throughgrepand formatted withpastecreates a focused list of changed.gmlfiles that will be processed by the formatter, which is perfect for the new weapon feature changes, nyan.
71-78: Improved error handling for formatting checks, nyan!
The updated conditional usingif ! git diff --quiet --exit-code; thencleanly captures any formatting discrepancies and provides detailed error messaging, nicely addressing previous concerns, nyan.
|
This PR is marked as stale, because it has been open for 7 days with no activity. |
1 participant
Summary by CodeRabbit
New Features
Chores