PaperMC
Add simpler EntityDamageByEntityEvent constructor
Is your feature request related to a problem?
The only non-deprecated EntityDamageByEntityEvent constructor requires creating maps to satisfy it, which result is just too long visually, especially when you don't need any special stuff in there.
Describe the solution you'd like.
A simpler EntityDamageByEntityEvent constructor, like the shortest one, just with boolean critical.
Describe alternatives you've considered.
I've made a simple EntityDamageByEntityEvent static factory, similar to underlying EntityDamageEvent constructor.
Other
While this is OK to sometimes use deprecated stuff, it's just not satisfying to, and might be rather unsafe in the future. Also, using static factory there seems to me like workaround rather than a real deal.
Unsure if this is a road we want to go down.
A few events here and there use this structure of more and more complex constructors. The same pattern exists in the rest of the API as well.
A simple helper function along the lines of
public static EntityDamageByEntityEvent entityDamageByEntityEvent(
@NotNull final Entity damager,
@NotNull final Entity damagee,
final @NotNull EntityDamageEvent.DamageCause damageCause,
final double damage,
final boolean isCritical
) {
return new EntityDamageByEntityEvent(
damager, damagee, damageCause,
new EnumMap<>(ImmutableMap.of(EntityDamageEvent.DamageModifier.BASE, damage)),
new EnumMap<>(ImmutableMap.of(EntityDamageEvent.DamageModifier.BASE, Functions.constant(-0.0))),
isCritical
);
}
somewhere in your code would also solve this instead of adding new constructors to the event.
Don't get me wrong, I see the utility such a patch would provide. I just don't know if it is the APIs place to provide utility constructors when everything can already be achieved with a rather short helper function.
I mean, short-handed constructor was available before the critical damage patch, and is available in both EntityDamageByBlockEvent and underlying EntityDamageEvent classes. So the absence of such in EntityDamageByEntity, and the fact that API requires to use deprecated stuff either way (as DamageModifier is deprecated too), just feels rather off.
Hmm yea I presume so. Would not be a large difference either so I guess nothing really in the way of it. I somehow thought it would be a somewhat larger diff, my bad.
This generally just seems like an issue that can just be solved yourself so easily, I'm not sure why this would be even really warranted... Not to mention, if this event was ever added onto it's possible that this utility method could become outdated and need to be deprecated itself.
So... eh...
This generally just seems like an issue that can just be solved yourself so easily, I'm not sure why this would be even really warranted...
I mean, yeah, it is easy to work with anyway. I'm actually absolutely fine with "long" constructors, but in this exact situation it's no good. I'll repeat my main points - it's inconsistent among other entity damage events, and current API requires you to use deprecated API either way. JavaDoc for DamageModifier literally says it's unsustainable to maintain.
If Paper is fine with DamageModifier API, then it should be undeprecated and the usage to be documented, and probably deprecating other "modifierless" constructors; otherwise EntityDamageByEntityEvent should have two constructors maintained - with and without modifiers.
Closing this since event constructors aren't considered API due to how often parameters may change, we wouldn't want to incentivize this by adding a shorter constructor/helper method. The method posted by lynx seems like a fair workaround that wouldn't require any changes on our end.
