Skip to content
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

Damage Pipeline Update #792

Open
wants to merge 36 commits into
base: 1.20.x
Choose a base branch
from
Open

Damage Pipeline Update #792

wants to merge 36 commits into from

Conversation

Caltinor
Copy link
Contributor

@Caltinor Caltinor commented Apr 10, 2024

This PR aims to create a more cohesive chain of events when entities are dealt damage.

For an in depth explanation of the need for this PR, please see This writeup by Tslat.

Overview

this PR adds two (2) new events, renames four (4), an override method for damage, and adds a protected field (damageContainer) to LivingEntity to track damage throughout the damage sequence.

DamageContainer Interface

This new field in LivingEntity tracks everything that happens during the sequence and is passed to each event. This means that for a single attack, no detail about the sequence from a prior step is obscured or inaccessible. For example, in later events, the modified and original damage are referenceable. Additionally values such as shield blocked damage, armor reduction, enchant reduction, absorption reduction, and mob effect reduction are all accessible (provided they are set at that point in the sequence). Since this container is provided to each event, it is also where modifications take place so that they persist between events.

When the sequence is complete, this object is nullified until a new attack is received. This is also a protected method to prevent access from the outside, since its sole purpose is to be a transient container for event-driven modifications happening internally.

The Sequence at a High Level

The "damage sequence" starts when the entity's hurt method is invoked and ends when damage is applied to the entity health. the sequence is as follows

  1. invocation of hurt
  2. invulnerability check. sequence ends if invulnerable <= EntityInvulnerabilityCheckEvent (new)
  3. cancel if on client, dead or dying, or damage is fire and entity is fire resistant
  4. first NeoForge hook to cancel or modify damage. <= EntityPreDamageEvent (formerly LivingAttackEvent)
  5. shield/blocking check and damage to shield <= DamageBlockEvent (formerly ShieldBlockEvent)
  6. freezing check
  7. invocation of actuallyHurt
  8. apply armor reduction <= ArmorHurtEvent (new)
  9. apply magic reduction
  10. apply absorption
  11. NeoForge hook to alter damage and cancel if zero <= IncomingDamageEvent (formerly LivingDamageEvent)
  12. final NeoForge hook to modify/cancel damage <= DamageTakenEvent (formerly LivingHurtEvent)
  13. entity health reduced by damage

Invulnerability Check

Currently the vanilla invulnerability check prevents modders from having any decision power on the receiving end of damage if vanilla decides the entity is invulnerable. The new event provides the following benefits:

  1. Modders always have a way to listen to an entity being attacked, even if the damage is nothing
  2. they can now override invulnerability
  3. There is now an earlier point to cancel damage.

EntityPreDamageEvent (formerly LivingAttackEvent)

Renamed to indicate this is the first stage in the damage sequence. This event is where most of the change should happen. Through DamageContainer#addModifier armor, enchant, mob effect, and absorption values are changed through the provided callbacks. Changing the damage in this event will have the most downstream effects.

Shield changes and the new DamageBlockEvent (formerly ShieldBlockEvent)

The current ShieldBlockEvent is nested inside vanilla checks as to whether the entity is blocking. Like the invulnerability check, this was decision power taken from modders. The new event is now part of the condition, so the damage blocked, the damage to the shield, and whether the player is considered blocking are all handled by the event.

ArmorHurtEvent (new) and Reductions from Armor, Magic, and Absorption

There is currently no way to modify damage to armor durabililty except to modify the damage to the entity through two different events. This event gives explicit control over the armor durability damage portion. Furthermore, the modifiers for damage reduction from armor, enchantments, mob effects, and absorption are applied in the DamageContainer instance.

IncomingDamageEvent (formerly LivingDamageEvent)

Renamed to indicate this is damage incoming to the entity and cannot be interrupted. The event is no longer cancellable for this reason. However, setting the damage to zero does still terminate the damage sequence. At this stage, all reductions have taken place and have been applied to the newDamage value. This is the last chance for users to change the final damage to the entity, but it will not revert and armor or shield durability losses.

DamageTakenEvent (formerly LivingHurtEvent)

Renamed to indicate this controls the damage that will be applied to the entity. This event is not cancellable, and receives an immutable version of the DamageContainer. This event works more as a trigger for events that need to fire when an entity takes damage AND needs to know the amount of damage received.

added ILivingEntityExtension#onDamageTaken

a default noop method that executes after the damage sequence has finished and allows custom entities a means to apply custom behavior during actuallyHurt without having to override the method.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

- function-based modifiers
- interface for damage container
- javadocs
@Caltinor
Copy link
Contributor Author

Updated with input from discussions on discord. Original post has been updated to reflect these changes as well.

@Shadows-of-Fire Shadows-of-Fire added enhancement New (or improvement to existing) feature or request breaking change Breaks binary compatibility 1.21 Targeted at Minecraft 1.21 labels Apr 15, 2024
@Shadows-of-Fire
Copy link
Contributor

Going to tentatively set this as targeting 1.21, due to the scope of other changes that are currently slated for 1.20.5. If we find ourselves in an accelerated timeline before the 20.5 stable release, we can revisit this decision.

@Shadows-of-Fire Shadows-of-Fire added this to the 1.21 Stable Release milestone Apr 15, 2024
@Shadows-of-Fire Shadows-of-Fire added 1.20.5 Targeted at Minecraft 1.20.5 and removed 1.21 Targeted at Minecraft 1.21 labels Apr 30, 2024
@Shadows-of-Fire Shadows-of-Fire removed this from the 1.21 Stable Release milestone Apr 30, 2024
@Shadows-of-Fire Shadows-of-Fire added 1.20.6 Targeted at Minecraft 1.20.6 and removed 1.20.5 Targeted at Minecraft 1.20.5 labels Apr 30, 2024
@Technici4n Technici4n added this to the 20.6 Stable Release milestone May 15, 2024
Copy link
Contributor

@Shadows-of-Fire Shadows-of-Fire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have time to finish all the files atm (I got about halfway through) but I'll submit this review set thus far. Lots of little changes that need to be made for sanity.

One thing I think needs to be looked at on a broad level is the event naming. The sequence doesn't have a coherent name set, and we have two clashing names that (at a glance) would appear to be the "initiating" event - EntityPreDamageEvent and IncomingDamageEvent. That might be better discussed on discord though.

|| this.invulnerable && !p_20122_.is(DamageTypeTags.BYPASSES_INVULNERABILITY) && !p_20122_.isCreativePlayer()
|| p_20122_.is(DamageTypeTags.IS_FIRE) && this.fireImmune()
|| p_20122_.is(DamageTypeTags.IS_FALL) && this.getType().is(EntityTypeTags.FALL_DAMAGE_IMMUNE);
+ return net.neoforged.neoforge.common.CommonHooks.onEntityInvulnerablityCheck(this, p_20122_, isVanillaInvulnerable);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempting to fire the event here is risky, but firing it at the call sites is untenable. At the least, I don't think we can reasonably provide the isVanillaInvulerable field since it is not necessarily correct - the common case of overrides to this method check additional conditions and then && them with super.isInvulnerableTo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only overrides where this occurs is:

  • Warden: checks if digging animation is active and damage source bypasses immunity
  • Ghast: checks entity invulnerability, same damage source check, and if the damage is not a deflected fireball
  • Breeze: damage source is in the breeze immune tag or source is from a breeze (self included)

I am of the opinion that the immunity bypass tags are compatible with this feature mainly because they are configurable. A user or modder still has control over the tag contents to handle immunity if it matters for these 3 mobs. The other aspects relate to reasonable invulnerabilities (breeze not affecting itself, warden (de)spawning). The Ghast fireball is the only other one that might be worth patching. I agree that patching each of these mobs for the sake of uniformity would be untenable in the long run though.

I don't think we can reasonably provide the isVanillaInvulerable field since it is not necessarily correct
Perhaps rename? This way it's clearer that it isn't absolute in its scope.

patches/net/minecraft/world/entity/LivingEntity.java.patch Outdated Show resolved Hide resolved
patches/net/minecraft/world/entity/LivingEntity.java.patch Outdated Show resolved Hide resolved
patches/net/minecraft/world/entity/LivingEntity.java.patch Outdated Show resolved Hide resolved
patches/net/minecraft/world/entity/LivingEntity.java.patch Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20.6 Targeted at Minecraft 1.20.6 breaking change Breaks binary compatibility enhancement New (or improvement to existing) feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants