-
Notifications
You must be signed in to change notification settings - Fork 290
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
Port ballistic damage type and preserve commit authorship info #1458
Port ballistic damage type and preserve commit authorship info #1458
Conversation
Currently, it's just 80% of cut, arithmetically rounded
Just set equivalent to stab damage, with arithmetical rounding
Iteminfo uses the 'front' damage unit of a damage instance for generating the damage and armor penetration displays for ammo. However, when items used relative or proportional, it unconditionally added an empty STAB damage unit in an attempt to compensate for the old damage loading system. Make adding this stab unit dependent on whether it actually will be empty or not, and revamp the checks for this to be more clear. Also, change the remaining 5 items that were getting away with using the old loading system because they did not change the values to use the new system (with the revamped checks, they now throw errors).
Ah! That's what I was missing, thanks. I'll go ahead and close #1398 in that case, and then set aside the BN-specific rebalances I had planned at a later date after this is merged.
If I was able to get cherry-pick to work right I would, but I have been unable to. Moreover, per #1298 (comment)
I do understand that cherry-pick would be preferable when feasible however, and will try do so if I can ever get the feature working. |
@@ -952,19 +952,25 @@ inline bool assign( const JsonObject &jo, const std::string &name, damage_instan | |||
float amount = 0.0f; | |||
float arpen = 0.0f; | |||
float dmg_mult = 1.0f; | |||
bool with_legacy = false; |
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.
Not sure if I get this part. It's always set to true if amount is >0
or explicit 0
is specified. Why is the name with_legacy
?
EDIT: Oh right, because it has this comment at the top that Github cut off because it's the 4th line.
Side note, working on follow-up PR to this, will be out and about today so many not be able to finish it until later on when I get home. |
Summary
SUMMARY: Features "Port over and implement ballistic damage type"
Purpose of change
I saw #1398 did not preserve commit authorship info. This is a version of that that does preserve that info.
Describe the solution
Cherry-pick the commits from
CleverRaven/Cataclysm-DDA#38912
CleverRaven/Cataclysm-DDA#45378
CleverRaven/Cataclysm-DDA#45336 (note! this fixes the issue mentioned in the PR body about the display)
Additional context
I don't particularly care or want this PR merged, but I would like if #1398 would use this branch and make whatever further changes it will with this as a basis, so that commit authorship info is preserved. And when you port things in the future, please peseve authorship info in commits.
Diff between this and #1398