Fix unpredictable jump damage from excessive velocity on unit collision #5468
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GoogleFrog
reviewed
Mar 5, 2025
|
|
||
| local damgeSpeed = math.sqrt((nx + tx*TANGENT_DAMAGE)^2 + (ny + ty*TANGENT_DAMAGE)^2 + (nz + tz*TANGENT_DAMAGE)^2) | ||
|
|
||
| local damgeSpeed = math.sqrt((nx + tx * TANGENT_DAMAGE) ^ 2 + (ny + ty * TANGENT_DAMAGE) ^ 2 + |
There was a problem hiding this comment.
This newline (and the one in UnitPreDamaged) seems needless.
|
My concern is that this PR seems to bound all fall damage. I like most of the whitespace changes, think that all the space before |
sprunk
reviewed
Mar 5, 2025
Comment on lines
+198
to
+213
| -- Cap damage to landing weapon damage if available | ||
| if maxLandingDamage[unitDefID] then | ||
| otherDamage = math.min(otherDamage, maxLandingDamage[unitDefID]) | ||
| else | ||
| -- Fallback cap for units without landing weapons | ||
| local unitHealth = UnitDefs[unitDefID].health | ||
| otherDamage = math.min(otherDamage, unitHealth * 0.2) -- Cap at 20% of max health | ||
| end | ||
|
|
||
| if maxLandingDamage[otherUnitDefID] then | ||
| myDamage = math.min(myDamage, maxLandingDamage[otherUnitDefID]) | ||
| else | ||
| -- Fallback cap for units without landing weapons | ||
| local otherUnitHealth = UnitDefs[otherUnitDefID].health | ||
| myDamage = math.min(myDamage, otherUnitHealth * 0.2) -- Cap at 20% of max health | ||
| end |
There was a problem hiding this comment.
Copy-pasted code and also there's a third copy at line 361. How about
local function CapLandingDamage(damage, unitDefID)
if maxLandingDamage[unitDefID] then
damage = math.min(damage, maxLandingDamage[unitDefID])
else
-- Fallback cap for units without landing weapons
local unitHealth = UnitDefs[unitDefID].health
damage = math.min(damage, unitHealth * 0.2) -- Cap at 20% of max health
end
endand then this chunk could become just
-- Cap damage to landing weapon damage if available
otherDamage = CapLandingDamage(otherDamage, unitDefID)
myDamage = CapLandingDamage(myDamage, otherUnitDefID)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Took a stab at fixing https://zero-k.info/Forum/Thread/38027, this approach just bounds max collision damage to the specified landing weapon. If there's no specified landing weapon, bounds the max val to 20% of health (arbitrary, happy to change this to whatever val is best).
If that's not ideal, I can also do it by clamping velocity as Googlefrog suggested in that thread.
NOTE: most of diff is from file formatting in line w the Lua style guide (first commit).
The commit to actually do the change is all in this commit
Highly recommend split view in GH review UI for viewing formatting changes.