Verify this is a more appropriate place for the function and there is no functional change.
Details
Details
- Reviewers
Silier - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP23964: Move TargetKilled function from Attacking.js-helper to KilledBy in cmpHealth.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2931/display/redirect
| binaries/data/mods/public/simulation/components/Health.js | ||
|---|---|---|
| 202 ↗ | (On Diff #13117) | also you can kill same entity multiple times |
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2934/display/redirect
Comment Actions
when updating tests, you should update all
| binaries/data/mods/public/simulation/components/tests/test_Damage.js | ||
|---|---|---|
| 95 ↗ | (On Diff #13120) | ^ -killed |
| 221 ↗ | (On Diff #13120) | ^ |
| 253 ↗ | (On Diff #13120) | ^ |
| 316 ↗ | (On Diff #13120) | ^ |
| 330 ↗ | (On Diff #13120) | ^ |
| 343 ↗ | (On Diff #13120) | ^ |
| 415 ↗ | (On Diff #13120) | ^ |
| 472 ↗ | (On Diff #13120) | ^ |
| 500 ↗ | (On Diff #13120) | ^ |
| 515 ↗ | (On Diff #13120) | ^ |
Comment Actions
when you are removing killed already, it would be preferable to destroy the object and send just that one number
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2948/display/redirect
Comment Actions
-we cannot tell if entity was killed by current attack without checking its health points, but this information was not used anywhere else.
+we can call takedamage from more places and statistics are covered
?TargetKilled might be renamed to something more appropriate as it is called on dying entity
| binaries/data/mods/public/simulation/components/Health.js | ||
|---|---|---|
| 215 ↗ | (On Diff #13171) | why? this is redundant variable |
| binaries/data/mods/public/simulation/components/Health.js | ||
|---|---|---|
| 215–221 ↗ | (On Diff #13185) | Actually I would say we would need to take the attackerOwner when the attack was performed (i.e. when the projectile was fired) instead of after ownership transfer. But that is out of scope for sure. |
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:154:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
CFixedVector2D(-halfSize.X, -halfSize.Y),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
builderr-release-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:154:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
CFixedVector2D(-halfSize.X, -halfSize.Y),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbolsLink to build: https://jenkins.wildfiregames.com/job/macos-differential/1334/display/redirect