I'd probably make the delay (which currently can already be 0) non-optional, so you don't have to worry about it not being there.

(It would be easy to merge with D451.)

@Mate-86:

I think you should do the work only in the damage component (it's perhaps time to add splash for melee attack too).
It seems also we can include #1910 in that one: a delay time and a repeat time?

I wonder also if we couldn't do them for single target too (as something like poisoning)?

I will look at the graphical thing.

edit: I will think at that again...

In D748#29339, @leper wrote:

I'd probably make the delay (which currently can already be 0) non-optional, so you don't have to worry about it not being there.

If I remove optional on delay then I get errors: "RelaxNGValidator: Validation error: (null):0: Extra element in interleave ..."

In D748#30866, @fatherbushido wrote:

@Mate-86:

I think you should do the work only in the damage component (it's perhaps time to add splash for melee attack too).

That's fine but do you mean the graphical part can be done solely in Damage component or do you mean excluding the graphical part from this ticket?

It seems also we can include #1910 in that one: a delay time and a repeat time?

#1910 is DeathDamage and already merged. Don't you mean including #1912 which is "Continuous damage after hit detection"?

I wonder also if we couldn't do them for single target too (as something like poisoning)?

Doing the delayed damage for a single target can be done but this must be well specified. Are those units affected which present when the projectile hits the ground (eg. poisoned and suffer later) or those which are present only after a delayed time (eg. grenade exploding delayed)?

I will look at the graphical thing.

edit: I will think at that again...

That would be great. The delayed damage without the delayed animation looks a bit lame.

Did you update the xml templates accordingly after making the tag non optional ?

In D748#31188, @Stan wrote:

Did you update the xml templates accordingly after making the tag non optional ?

Nope. Do you think I should update all the occurrences for existing unit templates (I've found 5 so far)?

If you make a field non optional you have to put it in every template

Back to this.

I was a bit confused with the design and what was really wanted.
So to sum up, the use case would be "Grenade". So the projectile hit the target or the ground and then cause damage after some delay.

The usecase makes me feel it should indeed be an area (splash) damage.

In the current diff, we launch the graphical animation, then call the collision detection with the delay.
So:

• we can't do a damage then do a delayed damage.
• if we let value for the not slash damage, that thing will be done also with a delayed damage.

I am fine with that design (there are other possibilities). In that case, there is no need to let the delay value in the splash schema (you can put it one level higher). And making it mandatory as leper said.

For the animation (#1909) I would suggest to split that. It's the complicated part.
We can just focus on the sound right now.
Btw, I notice a defect about that. We call the impact sound with the ent which is perhaps dead meanwhile.
That should perhaps be rethink. (Like parsing the sound name and calling the SoundManager component instead of Sound component.)
The other possibility could be to add an impact sound to the target entity. (There is the same question for the impact animation).
Though that's not exactly a good idea as in the delayed case we want to have an explosion sound even if we don't hit anything.

So except fixing the no sound when attacker is dead issue, I think we should add an attack_explosion or attack_delayed sound group which will be call at the same place than cmpProjectileManager->PlayProjectileHitAnimation(data.attacker, data.position); or even just at the start of MissileHit.

@Stan @fatherbushido Thanks for the comments! I've updated the ticket description, please have a look! I'm checking the sound change and uploading a new diff soon.

I'm wondering if you can't reduce that diff by using template inheritance for some of them.

I am fine with your implementation.
(We have to know that:

• it will do splash damage if enabled then it will try to hit a single unit if hit (So it's basically adding a delay to the hit detection to our current system.)
• we can't do damage before the delay

Just for the sum up)
We will keep the 0 delay for the onager (catapult) in the game, I guess we expect immediate damage for it. I don't really see any usecase in the public mod. Though we could think of some easter egg.

There is still the sound part to rework.

(I will also open a ticket for the attack_impact bug)

(That sound thing is a bit annoying)
Don t hesitate to suggest other design.

In D748#33256, @fatherbushido wrote:

(That sound thing is a bit annoying)
Don t hesitate to suggest other design.

OK, shall I raise a ticket for implementing this new cpp function to play sound based on the position (and pass attack_impact sound name to damage component too)? It's enough to create the Differential ticket and no need for new Trac ticket right?

(Well, I fixed some stuff sooner than expected)

! In D748#33385, @Mate-86 wrote:
OK, shall I raise a ticket for implementing this new cpp function to play sound based on the position (and pass attack_impact sound name to damage component too)? It's enough to create the Differential ticket and no need for new Trac ticket right?

Yes, let's keep that for another diff, you can open a ticket if you don't have time to work on it (or idea to fix it).

Thx for the patch and for the incoming ones.

LOL nice test evidence video! :D