-
-
Notifications
You must be signed in to change notification settings - Fork 342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Cause Enhancements #712
Conversation
public static final DamageType FIRE = null; | ||
public static final DamageType MAGIC = null; | ||
public static final DamageType PROJECTILE = null; | ||
public static final DamageType PLUGIN = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the name CUSTOM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed - if a mod uses a new damage type, it would likely use this one, and mods != plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a mod uses a new damage type, it wouldn't be PLUGIN
, it would have it's own id
and everything.
Would/Could these tie into your PR?
|
BlockMoveEvent, EntityExplosionEvent and EntityDeathEvent(or maybe a separate event for breaking Vehicles/Hanging or others that have no health) are not Cancellable. |
How would you prevent an entity from dying when it already has 0 health? Wouldn't you rather listen for the entity damage event that would kill the entity and prevent damage? |
import org.apache.commons.lang3.tuple.Pair; | ||
|
||
/** | ||
* A tuple of objects. This can be considered a {@link Pair} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then what's wrong with ImmutablePair?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That name works as well, however, using ImmutablePair still retains the method setValue
which I don't want to ever include because of the simple fact that calling it equates to getting a giant UnsupportedOperationException
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I mean the ImmutablePair class in Guava.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I mean the ImmutablePair class in Guava.
Why can't I find it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's actually from commons-lang https://commons.apache.org/proper/commons-lang/javadocs/api-3.1/org/apache/commons/lang3/tuple/ImmutablePair.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which is what I'm avoiding because of the comment I made three comments above this one.
I couldn't find an EntityDamageEvent. |
https://github.com/SpongePowered/SpongeAPI/pull/712/files#diff-775b410055a6c5940497d25e4bcdff8dR43 |
So this one will be fired for non Living Entities too? |
That is the point of any |
After a lengthy discussion with @bloodmc and @modwizcode, I've come to the conclusion of the following agreements for this PR:
|
abbb082
to
4a7cde7
Compare
"Do not go gentle into that good cause; Old reason should burn and rave at close of event. Rage, rage against the dying of the cause." Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>
This has been merged into the refactor/event-names branch. |
So far,
Cause
as it stands is somewhat confusing with the inclusion ofReason
such that it isn't clear as to what should be considered aCause
and what should be considered aReason
.This PR aims to achieve the following:
Cause
becomes a container object of all related objects associated with the cause for aCauseTracked
eventCause
, the closer the association to the direct cause, the lower the index that object will beEntityDamageEvent
andDamageSource
s while keeping in line with the primaryCause
goal. Any additional associated objects aiding the directDamageSource
are included in theCause
associated with theEntityDamageEvent
EntityPreDamageEvent
where the damage calculations for modifying the raw damage are included, this is primarily used for modifying the outgoing damage BEFORE theEntityDamageEvent
is fired, this is equivalent to having an event to modify the damage aPlayer
will output when using anItemStack
ofItemTypes.DIAMOND_SWORD
such that all modifiers are included (such as the enchantments on the sword, the potion effects, and any otherAttributeModifier
s or alternativeDamageModifier
s relating to the final raw damage output from thePlayer
CauseTrack
ed events, theCause
now is always available, but aCause.empty()
may be includedSo what does this really mean for
CauseTracked
events?What this means is that a vanilla interaction throwing a
CauseTracked
event will have as much information as possible within theCause
. With the following addition toCause
:a plugin is easily able to determine whether a
BlockState
was the cause for an event, aPlayer
, or if aTileEntity
was part of the mix.The one issue with
Cause
previously was that it was always optional. Passing in a null to theSpongeEventFactory
is that theCause
is always optional, even when it should not be. In the PR, I change this to whereCause
can be "empty". Some might think: Wait! Isn't that makingCause
just become a similar object toOptional
? To which the answer is yes. The one issue withOptional
however is thatOptional
can only hold a single value. There is no way to hold multiple values, except if theOptional
is declared like so:Optional<Object[]>
, which eliminates any utility methods provided bygetFirst(Class<T>)
andgetAllOf(Class<T>)
fromCause
.Ok, so where does this come into play with actual events? I want to know the cause for a
BlockChangeEvent
!Well, see, this is where the uniqueness comes into play. Since we always have a
Cause
, we can always callgetFirst(Player.class)
which will return a presentPlayer
if and only if aPlayer
was involved in theBlockChangeEvent
. How the objects are ordered in theCause
is simple: The direct cause of the event comes first, any additional "helpers" that aid the root cause are indexed afterwards.I'll give an example here of how a
BlockChangeEvent
could have the order of objects in the cause, for the case of aPlayer
placing aItemTypes.SAPPLING
.First, the
Player
is the root cause, because the player is right clicking with theItemStack
.Second, the
ItemStack
follows because theItemStack
is a part of the "reason" why theBlockChangeEvent
is being called.Now, what I haven't seen is how this affects something like
EntityDamageEvent
, care to elaborate?Of course! With an
EntityDamageEvent
, I'll have to pull the same description from #707What HAS been achieved however, is slightly different. Instead of just including the associated
Entity
as a root to theCause
, aDamageSource
is now the root cause. ADamageSource
is essentially a wrapper around any object with a descriptiveDamageType
with various attributes of theDamageSource
including whether the damage is absolute, ignores armor, magical, explosive, or even scaled with difficulty. The includedDamageSource
s in this PR facilitate understanding the type of damage being dealt, and the parties involved in theDamageSource
.I'm a little confused about
DamageSource
, can you give me an example?Of course! Say you have an
EntityPreDamageEvent
:The
DamageSource
in this case is actually aProjectileDamageSource
, where theProjectile
is anArrow
, and theProjectileSource
is aSkeleton
. Given that theProjectileDamageSource
is the root cause to theEntityPreDamageEvent
, we can then use this information to know that theCause
should have a relatedItemStack
used as a bow to shoot the Arrow. With the bow, we know that it can have noEnchantment
s or it can have someEnchantment
s. Of course, with these objects in theCause
, it makes it plain and clear to use them:Holy cow batman! Why so complicated?!
Unfortunately, there is A LOT that goes into an
EntityDamageEvent
, even more so if you want to be able to manipulate the damage coming in, essentially theEntityPreDamageEvent
. To fully unlock the power available with this, the possibilities to associate newDamageModifier
s with a damage event are nigh limitless. Since we can expose the underlying functions, as they placed in order from vanilla mechanics, we can successfully interpret that anEntityPreDamageEvent
is augmented not only by theSkeleton
, but also that theItemStack
used by theSkeleton
affects the total damage applied onto theArrow
. With that in mind, we can also assume that we can interpret the followingEntityDamageEvent
with similar processing forDamageModifier
s when anEntity
is being damaged, dictating the final damage actually being applied to theEntity
, and not just interpret the raw incoming damage.Of course, there are so many factors that can go into the
EntityDamageEvent
that I can't possibly explain it all in a simple code snippet, but I can explain in layman terms here.With the
EntityDamageEvent
being fired only while theEntity
is being damaged, we can gather allDamageModifier
s that would further process the raw incoming damage, such asItemStack
s that are considered to be armor,PotionEffect
s that are absorbing damage, heck, we can even apply our own customDamageModifier
s if we had a party plugin that had a "reducing damage" effect based on the number of players joined in the party! All of this is as extensible as possible.Ok... well, it's clear that you've given a lot of thought about
EntityDamageEvent
, but what about otherCauseTracked
events?Similar to the
EntityDamageEvent
andEntityPreDamageEvent
, we can safely say that mostCauseTracked
events are going to receive similar treatment with regards to adding additional sources/causes/reasons whatever you want to call them. So far, I've only been able to seriously work on the damage events, however, my thought process is that withEntitySpawnEvent
, it is very easy to apply the same idea of having the following:EntitySpawnEvent
:SpawnCause
: An abstract root cause that aids in helping further understand theCause
for why anEntity
is being spawnedSpawnType
: AnotherCatalogType
that aims to further simplify the type ofSpawnCause
, more specifically, when anEntity
is spawned by aMobSpawner
, theSpawnCause
would be an instance of aMobSpawnerSpawnCause
with the associatedMobSpawnerData
that defined theEntity
to be spawnedEntityTeleportEvent
:TeleportType
: Yet again, a helperCatalogType
that aims to make it understandable why anEntity
is being teleportedTeleportCause
: Another root cause for theEntity
to be teleporting, can range fromEntityTeleportCause
(anEnderman
teleporting due to rain),TeleporterTeleportCause
(a portal performing the teleport as per vanilla mechanics)The list will grow as the PR is worked on and time goes on.
So, will this make it easy for me to know if a
BlockChangeEvent
was caused by aPlayer
using some bonemeal on a sapling?In a short answer: yes.
The long answer: The
Cause
would indeed have aPlayer
object, and the secondarycause
would be theItemStack
that "aided" in the cause for theBlockChangeEvent
to take place. What is better is that with the knowledge of thePlayer
, theItemStack
, and any other objects placed into theCause
, it becomes very simple for you to decide on changing theBlockChangeEvent
for whatever purpose.How about throwing custom events? I don't really understand what I'm supposed to do here if I want to allow plugins to hook into my custom
EntityTeleportEvent
.To be honest, that is why I simplified
Cause
to replicateOptional
. When you are wanting to teleport anEntity
with aCause
, you can include the root cause, say theEntity
called aCommand
, and theCommand
was entered through aSign
, you now have two objects to include in yourCause
. However, let's say you wanted to expand and provide some customTeleportCause
s, you could extendTeleportCause
and provide the extendedCause
to be theSign
, and likewise you could include your own custom marker object so that in the event you have a listener forEntityTeleportEvent
, you can safely ignore the event if your custom marker object is included.How about X event?
Well, as I said before, the PR is still a work in progress, so I haven't finished designing the
Cause
for everyCauseTracked
event.