Uploaded image for project: 'Spigot'
  1. Spigot
  2. SPIGOT-5190

Superfluous EntityCombustEvent called (after EntityCombustByEntityEvent) when hitting something with Fire Aspect enchant


    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • None
    • None
    • Server is running on Windows 10. I don't really think it's relevant here as there's nothing environment-specific in the issue.

    • This server is running CraftBukkit version git-Spigot-9de398a-9c887d4 (MC: 1.14.4) (Implementing API version 1.14.4-R0.1-SNAPSHOT)
    • Yes

      When you hit an entity with a weapon with the Fire Aspect enchant, two EntityCombustByEntityEvents are called in succession from the EntityHuman class - the first event has a duration of 1, and the second has a duration of "enchant level times 4", so either 4 or 8 with vanilla enchantments. These events are sorta kinda expected and sound.

      After the first event (initial hit, duration 1), there is a call to Entity#setOnFire(int i, boolean callEvent) with the callEvent flag set to false. This is consistent with all of the other call sites, where an EntityCombustByBlockEvent or EntityCombustByEntityEvent is called before the call to setOnFire (except for the lava case in Entity, but there is a comment in the code about that). This is all fine.

      After the second event (damage-over-time effect, duration 8), there is a call to Entity#setOnFire(int i). This overload calls Entity#setOnFire(int i, boolean callEvent) with the callEvent flag set to true. This is inconsistent with the other call sites. In fact, all of the call sites for the Entity#setOnFire(int i) overload specifically do not call any combust events.

      I suspect this is an oversight in this commit, which set the callEvent flag to false for some of the previously mentioned call sites, including the one of the first event/initial hit in the EntityHuman class: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/843cee65f38547eed09067d7537c7441fc5647da#nms-patches/EntityHuman.patch

      I believe that the bug can be fixed very simply by explicitly calling Entity#setOnFire(int i, boolean callEvent) with the callEvent flag set to false at the other call site in EntityHuman, so that both call sites are consistent with each other.

      Because of this duplicate EntityCombustEvent, it is impossible infeasible to try to distinguish between e.g. combustion of a zombie from daylight and from a Fire Aspect-enchanted sword. As a result, Fire Aspect combustion is currently being cancelled in MobArena sessions because the code "thinks" that it's cancelling daylight combustion due to the superfluous EntityCombustEvent, even though it correctly filters out the preceeding EntityCombustByEntity event.

      To reproduce:

      1. Register a listener for EntityCombustEvent that reports the event class name (to see if it is the parent event or a child event) and the entity UUID (for traceability).
      2. Set the time to night to prevent noise from the daylight combust events.
      3. Ignite the ground below the zombie with Flint and Steel. Notice that the event listener reports just one event, an EntityCombustByBlockEvent.
      4. Hit a zombie with a weapon with the Fire Aspect enchantment. Notice that the event listener reports two events, first an EntityCombustByEntityEvent, then an EntityCombustEvent - this is the bug.

            Unassigned Unassigned
            garbagemule garbagemule
            1 Vote for this issue
            1 Start watching this issue