Opened 2 years ago

Closed 15 months ago

Last modified 14 months ago

#3610 closed enhancement (fixed)

[PATCH] Projectiles do not cause any damage if the shooter dies

Reported by: vikas MK Owned by: Leander Hemelhof
Priority: Nice to Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: leper Patch:

Description (last modified by leper)

Currently, projectile Damage depends on the shooter being alive.

This should not be the case and to achieve that some of the related logic should be moved from simulation/components/Attack.js to cmpDamage (in the same directory, which would be a system component created from simulation/helpers/Damage.js)

cmpDamage should keep information about who fired (player/entity) and when the damage should be applied. Most of this information is already stored in the current Damage helper object.

cmpDamage then uses a delay (see cmpTimer and the current usage of it in cmpAttack) to cause this damage.

Note that this ticket enables doing #1912 and #1911 (and possibly #1910) without any huge changes.

Attachments (15)

DamageAfterDeathOfAtacker.patch (24.7 KB) - added by Leander Hemelhof 17 months ago.
Changes the damage helper to a system component with the projectile hit handling logic
DamageAfterDeathOfAtacker_v2.patch (24.7 KB) - added by Leander Hemelhof 17 months ago.
Second version taking fatherbushido's regards into account
DamageAfterDeathOfAtacker_v2.2.patch (24.7 KB) - added by Leander Hemelhof 17 months ago.
Second version taking fatherbushido's regards into account
DamageAfterDeathOfAtacker_v3.patch (24.7 KB) - added by Leander Hemelhof 17 months ago.
Cleanup and fixing of the serialization issue
DamageAfterDeathOfAtacker_v4.patch (25.3 KB) - added by Leander Hemelhof 16 months ago.
Removal of the entityOwner map and some more cleanup
DamageAfterDeathOfAtacker_v5.patch (29.4 KB) - added by Leander Hemelhof 16 months ago.
Adds a test for the new damage component
DamageAfterDeathOfAtacker_v6.patch (31.5 KB) - added by Leander Hemelhof 16 months ago.
Added correct JSDoc to the damage component and some changes based on feedback.
DamageAfterDeathOfAtacker_v6-1.patch (33.4 KB) - added by fatherbushido 16 months ago.
DamageAfterDeathOfAtacker_v7.patch (20.2 KB) - added by Leander Hemelhof 16 months ago.
Fix for the test.
DamageAfterDeathOfAtacker_v8.patch (33.2 KB) - added by Leander Hemelhof 15 months ago.
Added the two missing files.
DamageAfterDeathOfAtacker_v9.patch (34.2 KB) - added by Leander Hemelhof 15 months ago.
Contains changes based on review.
DamageAfterDeathOfAtacker_v10.patch (34.2 KB) - added by Leander Hemelhof 15 months ago.
Fixed some mistakes and changes based on feedback.
DamageAfterDeathOfAtacker_v11.patch (34.2 KB) - added by Leander Hemelhof 15 months ago.
Some minor tweaks.
DamageAfterDeathOfAtacker_v12.patch (34.2 KB) - added by Leander Hemelhof 15 months ago.
Some minor changes to the JSDocs.
DamageAfterDeathOfAtacker_v13.patch (34.2 KB) - added by Leander Hemelhof 15 months ago.
Re-added the optional parameter indicators.

Download all attachments as: .zip

Change History (57)

comment:1 Changed 2 years ago by stanislas69

Have a look at #1910. Also #1911 #1909 might help you design it.

Last edited 2 years ago by stanislas69 (previous) (diff)

comment:2 Changed 2 years ago by wraitii

I think we need a CmpProjectileManager? that will handle all projectiles, either as specific entities or as something special. This would also provide a convenient interface to check if we are obstructed by walls or other buildings for say, arrows, or ultimately provide realistic physics for cannonballs...

comment:3 in reply to:  2 Changed 2 years ago by leper

Replying to wraitii:

I think we need a CmpProjectileManager? that will handle all projectiles, either as specific entities or as something special. This would also provide a convenient interface to check if we are obstructed by walls or other buildings for say, arrows, or ultimately provide realistic physics for cannonballs...

The projectile manager handles the visual only and is only used for rendering. Also this new component is usable for more things than just projectiles.

Having buildings block projectiles seems like asking for shitty performance, also I see no reason why that should be in a damage related component instead of eg cmpAttack. Physics for cannonballs should be a purely graphical thing and not related to the simulation at all.

comment:4 Changed 2 years ago by leper

Any updates?

comment:5 Changed 23 months ago by leper

Description: modified (diff)
Keywords: simple added; projectile system component removed
Summary: Moving missilehit from the shooter component to a system componentProjectiles do not cause any damage if the shooter dies

Note that there are some details (starting at 03:37) in this irc log: http://irclogs.wildfiregames.com/2015-11-15-QuakeNet-%230ad-dev.log

Changed 17 months ago by Leander Hemelhof

Changes the damage helper to a system component with the projectile hit handling logic

comment:6 Changed 17 months ago by Leander Hemelhof

Keywords: patch review added
Milestone: BacklogAlpha 21
Owner: set to Leander Hemelhof
Status: newassigned

Having taken some inspiration from the irc link linked above, this patch changes the damage helper to a system component wich also contains most of the projectile hit logic. This separates damage from the attacker, alowing projectiles to deal damage even after the attacker dies.

Completely moving the helper to a system component looked like the cleanest solution to me, and allows for future expansion, if needed.

comment:7 Changed 17 months ago by fatherbushido

We waited for this for a long time ! I will look at the code more deeply. I put it in the review queue. (Here are some quick useless formal remarks: Attack.js

  • L579: i guess the commentary is not needed

AttackDetection?.js

  • L66: perhaps you can use let

Damage.js

  • L6-8: could you skip the Init function ?
  • L27 : commentary perhaps not needed, or use JavaDoc? as you do in the other part of the code
  • L28 : the name of the function should start with Caps)

Changed 17 months ago by Leander Hemelhof

Second version taking fatherbushido's regards into account

Changed 17 months ago by Leander Hemelhof

Second version taking fatherbushido's regards into account

comment:8 Changed 17 months ago by Leander Hemelhof

Summary: Projectiles do not cause any damage if the shooter dies[PATCH] Projectiles do not cause any damage if the shooter dies

Thanks for the remarks !

I didn't change the TestCollision function at first because I just copied it over from the Attack component. This new patch changes the function name and changes the comment above to a JavaDoc?. It also deals with the other remarks.

Edit: Please ignore the _v2.2.patch. I seem to be unable to delete it.

Last edited 17 months ago by Leander Hemelhof (previous) (diff)

comment:9 Changed 17 months ago by fatherbushido

I tested it with a low projectile speed ballista and it has the expected behavior. Here are some quick remarks again

  • (for future reviewer) the patch apply to data/, not to binary/
  • AttackDetection.js L67-70 could perhaps be merged with a || or something like that
  • I understand why you need that entityOwners map but i wonder if we can in a way or another skip that ?

Moreover, there is a more serious issue pointed out by Sanderd17 (see 2016-07-04 irc logs). The entityOwners map should be in the Init else it will not be serialized (for multiplayer and saved games).

Then while you are at it, we can consider you are rewriting Damage, so you can remove depreciated for each loops.

Last edited 17 months ago by fatherbushido (previous) (diff)

Changed 17 months ago by Leander Hemelhof

Cleanup and fixing of the serialization issue

comment:10 Changed 17 months ago by Leander Hemelhof

This patch should have all the requested cleanups. I also hope that I have created this patch from the right directory this time.

I did some minor tweaks with the entityOwner map. The owner of the attacker is now sent as data in the timer and only added to the map if the attacker is dead when the projectile hits. The for each loops are regular for loops now. Please let me know if there is a more preferred way of changing those.

comment:11 Changed 17 months ago by fatherbushido

Thanks for working on this.

We (allow to) use for (let of ... instead of for (let i = 0; i < ents.length; ++i) in the code.

I will do a full review in 2 weeks (if nobody does it before).

Last edited 17 months ago by fatherbushido (previous) (diff)

comment:12 Changed 16 months ago by fatherbushido

Testing the patch, i got some errors like ERROR: Script message handler OnGlobalAttacked? failed ERROR: Error in timer on entity 1, IID 57, function MissileHit?: ReferenceError?: cmpAttackOwnership is not defined

But the logic is ok. And it works as expected in game for the damage part.

I suggest to get rid of this entityOwner map. I suggest you yo use the attackerOwner from PerformAttack? and add it in data in all MT_Attacked message and in data parsed in CauseDamage?, CauseSplashDamaged?, and as arg in AttackAlert? function. Tell me if i miss something.

(Then we will have to look if there is issue with Guard and AI (see Guard.js, UnitAI.js, AIProxy.js). But it should be ok.)

Changed 16 months ago by Leander Hemelhof

Removal of the entityOwner map and some more cleanup

comment:13 Changed 16 months ago by Leander Hemelhof

I tested that error and concluded that it only happens in games created before adding the entityOwner map to the init function, so that shouldn't happen anymore.

This patch should pass the attacker owner as an argument to AttackAlert in AttackDetection.js using the passed data. I did a few tests and everything seems to work at the moment.

All the for-loops in the Damage component should now also be in the form of "for (let foo of bar)"

comment:14 Changed 16 months ago by fatherbushido

Thanks for being so fast and working on that. That should be ok now.

(In binaries/data/mods/public/simulation/components/Attack.js, we can perhaps do let attackOwner = Engine.QueryInterface(this.entity, IID_Ownership).GetOwner() just before L484 as it is uses twice in the data stuff. But don't upload a new patch only for that.)

I will now check what happens with Guard and AI (if there could be some owner issue). Then we need to look at writting tests and finally get another team member overview.

comment:15 Changed 16 months ago by sanderd17

Keywords: rfc added; review removed

@LeanderH, I do really appreciate your work, but per the new SubmittingPatches guidelines, tests for altered and added simulation code should also be provided.

We were having more and more code stability issues, and by adding tests, we hope to be able to improve stability of code. Thanks a lot for wanting to work on this.

See f.e. #4121 on how to do this.

Last edited 16 months ago by sanderd17 (previous) (diff)

Changed 16 months ago by Leander Hemelhof

Adds a test for the new damage component

comment:16 Changed 16 months ago by Leander Hemelhof

This patch should implement a damage test, testing the changed functions. I rewrote it a couple of times, so please let me know if there are any traces left from previous versions (unneeded variables, ...).

Currently the test runs fine on my system.

Of course, all feedback is welcome.

comment:17 Changed 16 months ago by stanislas69

Thank you for working on this, some comments I made on IRC, only style related, a team programmer will probably have functionnality comments

<Stan> Not much to say on the above patch
<Stan> typo l23 on damage.js
<Stan> some vars could be let l203-204-205-206
<Stan> line 50 of Damage.js, maybe d could be replaced by something more meaningful
<Stan> (distance maybe ?)
<Stan> for functionnality and cleanliness I guess you have more knowledge elexis 

comment:18 Changed 16 months ago by fatherbushido

Thank you very much for the patch. I will (pre)review that quickly. (We need to well document components/interfaces and use the adhoc jsdoc tags in the new Damage component wich was missing in the old data helper, but don't upload a new patch for that before the review).

comment:19 Changed 16 months ago by elexis

  • Nice line-ending awareness ;)

Some style things:

  • Damage.js:
  • L11 missing spaces
  • L20 unneeded parenthesis
  • L23 use actual JSdoc syntax
  • L33 newline
  • L44 unneeded parenthesis
  • L116 unneeded comment
  • Use let everywhere
  • L145, L150 if( -> if (
  • L147 148 can be merged
  • L164 lines should end with a semicolon or operator
  • L196 and following unneeded comments
  • L214 various missing spaces
  • L236 use .map() or construct that using the number of players, perhaps using new Array().fill()
  • test_Damage.js
  • L27 missing spaces

I'm happy to leave the logic checks up to fatherbushido :p

comment:20 Changed 16 months ago by elexis

Few more things:

test_Damage.js:

  • Omit the unneeded return using () => ({ "foo": bar })

Damage.js:

  • L18 Would move the first argument also to a newline

Attack.js:

  • L555 too much indentation
  • L569 should there be a +?

comment:21 Changed 16 months ago by fatherbushido

(Pre)Review of the patch

From the logic point of view :

Attack.js

  • L484 Perhaps you have to add a check. So an early return or throw an error like
    let cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
    if (cmpOwnership)
        error("!PerfromAttack called with an invalid ownership")
    else 
        let attackerOwner = cmpOwnership.GetOwner();
    
  • L569 data.range = this.template.Ranged.Splash.Range; -> data.radius = this.template.Ranged.Splash.Range;

AttackDetection.js

  • L61 same Ownership check as in attack ? (You can also use cmpPlayerTarget = QueryOwnerInterface(target) to merge some lines)
  • perhaps use let playerID = cmpPlayer.GetPlayerID() as it is use three times sin that function.
  • L65 can be remove

From the formal point of view :

I know that most of the formal issues were not introduced by you, but while at moving that part of the code, we have to fix them.

  • see all elexis remarks
  • in objects use space after { and before }
  • when a function have a JSDoc comment, fix it in using the right JSDoc tags, like (from JSDoc doc):
    /**
     * Assign the project to an employee.
     * @param {Object} employee - The employee who is responsible for the project.
     * @param {string} employee.name - The name of the employee.
     * @param {string} employee.department - The employee's department.
     */
    
  • Btw notice that the comment of Damage.TestCollision? "point = <Vector2D>" is wrong as point is a Vector3D (or an object { "x": number, "y": number, "z": number })
  • Look at the message documentation in binaries/data/mods/public/simulation/components/interfaces/Damage.js and binaries/data/mods/public/simulation/components/interfaces/Attack.js

Review of the test

Here you can perhaps only focus on the Damage part and not the whole process. Imo you can only test the Damage functions: mock almost all and just parse data through CauseSplashDamage? and CauseDamage? and check that the sent message is the good.

Last edited 16 months ago by fatherbushido (previous) (diff)

Changed 16 months ago by Leander Hemelhof

Added correct JSDoc to the damage component and some changes based on feedback.

comment:22 Changed 16 months ago by Leander Hemelhof

First of all, thanks for the feedback Stan, elexis and fatherbushido. This patch should (hopefully) implement the requested changes.

Another change I made is to the TargetKilled? function (L271 of Damage.js). I added the attackerOwner as parameter to allow the retreval of the StatisticsTracker? component from the attacker's player entity, even when the attacker is dead.

comment:23 Changed 16 months ago by stanislas69

let cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership); 
let attackerOwner; 
if (!cmpOwnership)
    error("PerformAttack called with an invalid ownership component"); 
else 
    attackerOwner = cmpOwnership.GetOwner(); 

use early return instead of displaying an error there is no need to continue

let cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership); 
if (!cmpOwnership)
    return;
let attackerOwner;  = cmpOwnership.GetOwner(); 

For the rest I can't really tell for now, so I'd leave that to fatherbushido, and elexis

comment:24 Changed 16 months ago by fatherbushido

Thank you for the update of the patch, that sounds very good, i will do (i hope tommorow) a (last) review of it ; if it's ok we will put it in the review queue.

EDIT:

i modified directly some stuff in the last patch:

  • take care in the version i upload there is not the test
  • i will now check the docs, check the tests and do a final test.

Say me if you see something wrong.

Last edited 16 months ago by fatherbushido (previous) (diff)

Changed 16 months ago by fatherbushido

comment:25 Changed 16 months ago by Leander Hemelhof

That patch looks fine to me. I don't see anything wrong with it. I do have noticed that my last patch broke the test. The only thing missing is a mock for the function IsInWorld? in the target's position component. That should be fixed in my next patch. I also decided to merge two if-statements in AttackDetection?.js (L64 and L67 in v6-1) to make the code look cleaner.

Changed 16 months ago by Leander Hemelhof

Fix for the test.

comment:26 Changed 16 months ago by fatherbushido

Thx for your patience and your work!

  • You forgot to add Damage component and Damage interface in the last patch
  • Test seems ok (I would not have do it like that but it's only a test)
  • In game tests seems ok

When the 2 files will be added, change the tag rfc into review.

Last edited 16 months ago by fatherbushido (previous) (diff)

Changed 15 months ago by Leander Hemelhof

Added the two missing files.

comment:27 Changed 15 months ago by Leander Hemelhof

Keywords: review added; rfc removed

This should have added the two missing files.

comment:28 Changed 15 months ago by leper

Keywords: rfc added; review removed

Note that some of the issues are in the code moved, and should not be part of this patch, but of a followup patch that. I also didn't explicitly check for issues introduced in the moved code (didn't diff the removed against the added code).

  • Attack.prototype.PerformAttack
    • should not abort if there is no ownership (and if it is -1 there is some bug somewhere, so we should expose it by just making something fail loudly), this is a behaviour change that should not be part of this patch (not that I see any value in having that in another).
  • AttackDetection.prototype.AttackAlert
    • shouldn't return if cmpPlayer is not set, but error out (as in just omit the check).
    • attackerOwner should only be used as a fallback in case the attacker does not exist anymore (so in case !cmpAttackerOwnership (or .GetOwner() == -1), as in the current patch the attacker parameter is unused).
    • Check for owner and not for equivalence of components (shouldn't matter, but the previous code expressed intent)
  • Damage.prototype.TestCollision
    • Checks for !targetPosition twice, remove the latter check.
    • Since we have if (foo) return bar; the else (and the additional indentation of that block) isn't needed.
  • Damage.prototype.MissileHit
    • playersToDamage should most likely be inlined or at least the if setting it should do some inlining QueryPlayerIDInterface(data.attackerOwner).GetEnemies().
    • Should we really return if !targetPosition instead of only failing in the one case where that is actually needed?
    • The cmpProjectileManager.RemoveProjectile(data.projectileId); call in the loop is wrong, and that is done for both the if and the else branch, so move that there. This also means that the early return likely doesn't remove the projectile.
    • Also the loop seems wrong as it will damage all nearby units, unless I am missing something.
  • Damage.prototype.CauseSplashDamage missing spaces after some colons in the data passed to CauseDamage.
  • Damage.prototype.CauseDamage only uses cmpHealth in one if, but returns if it isn't present (though IIRC the DamageReceiver code will likely fail somewhere, but that should be fixed there IMO; still dealing damage to something without health sounds like an issue, so that should be exposed)
  • Damage.prototype.EntitiesNearPoint
    • should probably use a new function in cmpPlayerManager to get all the player ids instead of using PlayerManager.prototype.GetAllPlayerEntities which should be removed. (Though this is definitely out of scope for this ticket)
    • The naming of variables should be fixed, but L260-262 should just be inlined.
  • Damage.prototype.TargetKilled should use the owner of killerEntity (can we get a better name for that?) before falling back on attackerOwner.
  • A test utilizing cmpAttack to test whether the damage is dealt when the attacker is killed after firing would be great (as that could be used to test that the current code is broken, and that this patch fixes it). (Basically a regression test)

Nice work so far, keep it up.

comment:29 Changed 15 months ago by Leander Hemelhof

Thanks for the review. This patch should hopefully contain the requested tweaks.

As for the loop in Damage.prototype.MissileHit, The enemies of the attacker are passed to the ExecuteQueryAroundPos? function, so it looks like it only targets the enemies in range.

Changed 15 months ago by Leander Hemelhof

Contains changes based on review.

comment:30 Changed 15 months ago by leper

  • Attack.prototype.PerformAttack
    • The owner != -1 check in the capture case of Attack.prototype.PerformAttack should be kept (for that case only).
    • L586 missing space after :.
  • AttackDetection.prototype.AttackAlert
    • ?: has lower operator precedence than && so remove the parenthesis.
    • Use playerId instead of Engine.QueryInterface(this.entity, IID_Ownership).GetOwner() in the if. (The point of this ticket is not to add behaviour changes, apart from the one that is done by the move to a system component)
    • Also move the cmpAttackerOwnership init/decl to where it was previously.
  • Damage.prototype.Init put the { on a new line, and the }; on another line (same as eg cmpWonder).
  • Damage.prototype.TestCollision
    • remove the braces for the if (foo) return bar;, add a blank line after the return.
    • I'd remove the empty line after the angle decl, and merge the distance lines.
  • Damage.prototype.MissileHit
    • Given that the projectileId is not used in any of the called functions, the removal of the projectile could happen right at the start, thus removing the duplication of those two removals.
    • Yes, the loop only damages entities in range, but it does target all of them, not only one (which means actually not hitting the wanted unit, but all at the position; regardless of splash damage!). (This could be handled in another ticket)
    • As the removal of the projectile now happens at the start, we could just return in the if (testCollision) on L112, then we don't need the else or the additional level of indentation.
  • Damage.prototype.CauseDamage
    • move cmpHealth closer to where it is used.
    • add an empty line before Engine.PostMessage
  • Damage.prototype.TargetKilled
    • Missing let
    • Typo in the variable name
    • Could use let attackerOwner = Engine.QueryInterface(attacker, IID_Ownership).GetOwner() || attackerOwner; 'iff' that doesn't error in case the first thing fails.
  • It would be great if the regression test would actually be a test of cmpAttack instead of cmpDamage, but that doesn't work since we'd have to change the test if we'd add that before the rest (to verfiy that it is indeed broken before the patch and fixed afterwards), so ignore this bullet point.

Changed 15 months ago by Leander Hemelhof

Fixed some mistakes and changes based on feedback.

comment:31 Changed 15 months ago by Leander Hemelhof

Thanks again. I tested the proposed change of the definition of attackerOwner in Damage.prototype.TargetKilled, but it does cause an error if the component is null. I just left it as it was, since that seems to be the cleanest I can make it.

comment:32 Changed 15 months ago by leper

  • AttackDetection.prototype.AttackAlert
    • Why did you remove cmpTargetOwnership instead of using that (that one check was nice as it was before the patch)?
  • Damage.prototype.MissileHit
    • Adding a comment before L109 that this is checking for direct hits would be nice. eg // Direct hits
    • 'if(' -> 'if (' as per wiki:Coding_Conventions
  • Damage.prototype.EntitiesNearPoint
    • The comment on L249 is invalid, as the next line doesn't use a dummy entity, and hasn't done so for quite some time, nuke that.
  • Damage.prototype.TargetKilled
    • Does not check for owner != -1 before using it, should fix that by using the same check as you already have in another place.

Looks good apart from that, I'll look into getting it committed this weekend.

Changed 15 months ago by Leander Hemelhof

Some minor tweaks.

comment:33 Changed 15 months ago by Leander Hemelhof

This patch should take care of those things. I reintroduced the original check in AttackDetection.prototype.AttackAlert, but I didn't reintroduce the variable because it's only used once.

comment:34 Changed 15 months ago by fatherbushido

If i might suggest some formal things:

Damage.js -L34 int -> number -L36 int -> number -L70 { 'hack': <float>, 'pierce': <float>, 'crush': <float> } -> { "hack": number, "pierce": number, "crush": number } -L146 { 'hack': <float>, 'pierce': <float>, 'crush': <float> } -> { "hack": number, "pierce": number, "crush": number } -L150 [data.playersToDamage] -> data.playersToDamage -L201 { 'hack': <float>, 'pierce': <float>, 'crush': <float> } -> { "hack": number, "pierce": number, "crush": number } -L204 float -> number -L234 [players] -> players

Last edited 15 months ago by fatherbushido (previous) (diff)

Changed 15 months ago by Leander Hemelhof

Some minor changes to the JSDocs.

comment:35 Changed 15 months ago by Leander Hemelhof

Thanks for the feedback. This patch should implement those changes. So I don't need to specify which parameters are optional using square brackets?

Last edited 15 months ago by fatherbushido (previous) (diff)

comment:36 Changed 15 months ago by fatherbushido

oh sorry for that, let the brackets so !

Changed 15 months ago by Leander Hemelhof

Re-added the optional parameter indicators.

comment:37 Changed 15 months ago by Leander Hemelhof

No problem. The coding conventions don't really specify anything about optional parameters, so I wanted to be sure. This patch should add them back.

comment:38 Changed 15 months ago by leper

In 18624:

Add regression test for damage being dealt if the attacker dies. Based on patch by LeanderH. Refs #3610.

comment:39 Changed 15 months ago by leper

Resolution: fixed
Status: assignedclosed

In 18625:

Deal ranged damage even if the attacker dies. Patch by LeanderH. Fixes #3610.

Create a Damage system component from the Damage helper object and parts of
the Attack component. This fixes the issue by making ranged damage independent
of the attacking entity.

While there fix the issue of damaging all nearby entities in case the actual
target was not hit, instead of only a single one.

comment:40 Changed 15 months ago by leper

Keywords: simple rfc removed

Thanks for the patch.

I split out the regression test (and added a comment about what we are testing for) into a commit of its own. Also sorted the scripts that are loaded.

In the second patch I fixed the removal of projectiles (after checking the code again, sorry for not having done that before) as misses should leave the projectile, which will be removed after some delay by the ProjectileManager. In addition to that I fixed the case where the target isn't hit, but there are other possible targets nearby the location where the projectile hit, which previously caused all possible targets to be damaged (which would add splash damage without damage scaling to projectiles not having that property).

Can I look forward to you working on one of the related tickets listed in the description?

comment:41 Changed 15 months ago by elexis

bug -> #4169

comment:42 Changed 14 months ago by fatherbushido

In 18810:

Fix two issues related to splash damage: fix a mismatch between the string false and the false boolean, fix an issue with an omitted array of players. Noticed by Mate-86. Patch by leper. Refs #3610, #1910, r18645.

Note: See TracTickets for help on using tickets.