-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: custom stat scaling property for weapons #73
base: release/0.10.0.0
Are you sure you want to change the base?
Conversation
rebase onto #64 before continuing with this or we will have issues as i've moved the equipment struct to it's own file |
@sourcery-ai review |
Reviewer's Guide by SourceryThis pull request introduces a new stat scaling property for weapons. This property allows weapons to scale their stats based on the marine's stats. The changes involve adding a new Sequence diagram for weapon stat bonus calculationsequenceDiagram
participant Marine as TTRPG_stats
participant Weapon as equipment_struct
Marine->>Marine: melee_attack(weapon_slot)
Marine->>Marine: get_weapon_one_data()
Marine->>Marine: get_weapon_two_data()
Marine->>Marine: get_weapon_stat_bonus(weapon1)
activate Marine
Marine->>Weapon: access stat_scaling
Weapon-->>Marine: return scaling values
Marine->>Marine: calculate stat bonus
deactivate Marine
Marine->>Marine: get_weapon_stat_bonus(weapon2)
activate Marine
Marine->>Weapon: access stat_scaling
Weapon-->>Marine: return scaling values
Marine->>Marine: calculate stat bonus
deactivate Marine
Marine->>Marine: update melee_att
Class diagram showing weapon stat scaling changesclassDiagram
class equipment_struct {
+String type
+Number hp_mod
+String description
+Number damage_resistance_mod
+Number ranged_mod
+Number melee_mod
+Number armour_value
+Number attack
+Number melee_hands
+Number ranged_hands
+Number ammo
+Number range
+Number spli
+Number arp
+StatScaling stat_scaling
+equipment_struct(item_data, core_type, quality)
}
class TTRPG_stats {
+Number ranged_att
+Number melee_att
+Number weapon_skill
+Number strength
+melee_attack(weapon_slot)
+get_weapon_stat_bonus(weapon)
}
class StatScaling {
+Number strength
+Number dexterity
}
equipment_struct -- StatScaling
TTRPG_stats ..> equipment_struct : uses
note for StatScaling "New struct for weapon stat scaling"
note for TTRPG_stats "Added get_weapon_stat_bonus method"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @EttyKitty - I've reviewed your changes - here's some feedback:
Overall Comments:
- The get_weapon_stat_bonus function uses undefined variables 'stats' and 'stat_name' - these should be '_stats' and '_stat_name' to match the loop variable names
- Consider removing or updating the commented out explanation string code rather than leaving it as comments
Here's what I looked at during the review
- 🟡 General issues: 1 issue 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.
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)), "%#"); | ||
var explanation_string = "1"; |
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.
issue (bug_risk): Hardcoded "1" value appears to be temporary debug code
The explanation string construction was commented out and replaced with a hardcoded "1". This might affect game feedback and debugging capabilities.
Summary by Sourcery
New Features: