This PR essentially reverts @aevyrie's #16716 right? Are the arguments from that PR no longer valid?

The field entity: Entity encodes no semantic information about what the entity is used for, you can already tell that it's an Entity by the type signature!

Stands out to be the biggest argument to keep it as is.

I never signed off on that one. I think the arguments are reasonable, but not enough to sway me:

The question users will be asking is: "what Entity was this Event triggered for?". The natural way to ask that question in code is event.entity(). This is especially true in light of the EntityEvent naming. I'm of the opinion that "event entity-ness" in this context is significantly more relevant and useful than "event target-ness".

I believe that particular change was motivated by a desire to disambiguate between "observer entity", "target entity", and "original target entity". When considering those things as an internals developer, I can see the desire to treat them all as equals here. But in practice the "observer entity" is deeply niche. We don't need to disambiguate that for 99.99% of users, who just want to know what entity their EntityEvent was triggered on. event.entity() is the clearest, most direct way to handle that case. I'm content with developers who want the "observer entity" to need to understand the event.entity() vs event.observer() distinction ... but I don't think those developers will be confused either.

I do think that the churn is regrettable. But not so regrettable that I think we should hold back (what I consider to be) real clarity improvements.

#16716 was also back when we were using trigger: Trigger<Explode> terminology, where Trigger was more of its own "thing". Now that we are embracing "eventness" with event: On<Explode>, I think that further improves the clarity of event.entity().

Yeah the churn is mildly amusing for me since I am porting to 0.16 literally right now, and I just did the trigger.entity() -> trigger.target() thing, but it's an easy migration either way so I agree you should pick the "right" API here rather than worry about the churn.

The question users will be asking is: "what Entity was this Event triggered for?".

@cart what is this hypothesis based on? The change happened because people were getting confused about what entity that was.

It's pretty natural to want both the target and the listener, that happens all the time in UI/reactive code. Calling this entity removes any semantic information about the kind of entity this is - it's already trivially obvious that you have an Entity.

I'm of the opinion that "event entity-ness" in this context is significantly more relevant and useful than "event target-ness

When people discuss "what entity was this event triggered on", the answer is the event target, not the "event entity". Calling it an event target is some very standard terminology IME, and using the name .entity() removes all the semantics about the kind of entity it is, like aevyrie mentioned.

For me, it's not even about disambiguating between .entity() and .observer(), it's about what the .entity() is and what name makes sense for it in that context. It is the target entity of the event. As for API consistency, the name of the method that even triggers the event is .trigger_targets(). It is strange to me that "target" is then stripped away from the actual Event API.

To me, .entity() is not at all a clarity improvement. I would not be too surprised to see it renamed back to .target() yet again next cycle if this is released, so it just feels like a lot of churn to make this change at the end of the cycle when there doesn't seem to be broad consensus on it.

Was this discussion ever resolved? I see the PR was merged, but I also think people raise valid concerns that haven't gotten a response here (perhaps somewhere else?). Given that 0.16 changed it to target, I feel like, if there's still something to discuss here, we still have time to keep that name scheme, before 0.17 ships, if we want to?

I consider this to be resolved by #20731, which solves this in a variety of ways:

1. Entity events now include the entity target inside the event, and can give it whatever name makes the most sense for the event. As a pattern, we encourage developers to directly use the field to access the target entity.
2. Because the field is used directly, this also allows event authors to directly document the field, allowing more specific context.
3. This is less common, but if using the EntityEvent trait to access the target entity, it now uses EntityEvent::event_target to be less ambiguous.