Add timeDamaged to EntityDamageEvent

[Cryptite]

My finest work yet. Just a simple way to fetch the time an EntityDamageEvent happened.

Unsure if we want to use things like epoch time still or whether we're trying to use things like ticks or other time measurements; happy to update as needed if so.

[456dev]

is this different to calling System.currentTimeMillis(); in your event handler?
or maybe a stupid suggestion, if its useful here, wouldnt it be useful in every event, as an interface / smth

[electronicboy]

I mean, for most intents and purposes, the difference will be minimal unless you're posting that off async; time millis is generally fast enough, so it's not a big deal but I don't exactly see an inherient advantage here; isn't this one of the things that mojang stores a timestamp for the packet?

[Malfrador]

If there is ever API for DamageSources and the CombatTracker, I could see a timestamp added to each DamageSource being useful, because then time, involved entities, direction and type of each instance of damage received would be together.
But currently I fail to see the advantage over just listening to the event and storing the timestamp yourself.

Vanilla only tracks combat start and end time as well as the time since the last attack, not timestamps for individual damages.

[Cryptite]

is this different to calling System.currentTimeMillis(); in your event handler? or maybe a stupid suggestion, if its useful here, wouldnt it be useful in every event, as an interface / smth

The difference (for me) is that EntityDamageEvent is something that can be later accessed off an Entity with getLastDamageCause(), whereas most other events are instant and not stored.

But currently I fail to see the advantage over just listening to the event and storing the timestamp yourself.

Mostly it's because this is a trivial object addition to an event whereas one would need to start managing HashMaps etc for this which would certainly be (a little) more effort, memory cost, and the like. It's certainly not a vital addition but I could see it being useful for others. All else fails I'll just continue using it in my fork.

Like with above, considering that you can fetch getLastDamageCause() off an Entity at any point, being able to tell when that event happened can be useful.

[electronicboy]

This is generally the weird area of "I can get the why" but, It feels weird to accept this for just a single set of events, but it would also be weird to shove it for all events. I think this gets into the pattern where people for years have just wanted a general form of mechanism to attach data to event objects

[ComputerNerd100]

@Cryptite PR requires rebasing (see https://github.com/PaperMC/Paper/blob/master/CONTRIBUTING.md#rebasing-prs)

[SO-32]

After rebasing, tested PR. Confirmed working

[Owen1212055]

Yeah, I don't see the point in this. And in general, this API will need to be reworked for the whole proper damagecause system.

[Cryptite]

Yeah, I don't see the point in this. And in general, this API will need to be reworked for the whole proper damagecause system.

If there were a DamageCause class object, rather than just an enum, that would be a nice place to put something like this.

[lynxplay]

Yea I think I'll close this for now.

I 100% see the usecase for this, but yea, merging a temporary solution when we know we want a better one down the line seems rough for an API that wants to be backwards compatible if possible.

I am glad to see that the patch found a place in Slice however 😉