Obsidian-Bomb breaks wave progression clear-boss-before-next: true

[SoraShiunin]

Bug report

Short description

Having obsidian bomb skill on any boss will break "clear-boss-before-next: true" From the Discord convo here: https://discord.com/channels/363735561179103237/436983158416867341/1196026548092932176

Reproduction steps

1. Join an arena with /ma join
2. Progress through the arena
3. Wave proceeds when it shouldnt in boss wave.

Details

• MobArena version: MobArena v0.108
• Server version: Paper 1.20.4

Additional info

[NightmareFox]

Obsidian-bomb completely breaks the boss wave. The boss becomes a regular mob that does not even attack the player and does not clear wen the arena finishes.

[garbagemule]

This was one heck of an issue to track down, holy crap...

It turns out this is a regression introduced in 0.108 by this commit: https://github.com/garbagemule/MobArena/commit/4f78936716d6218a1cdc0d6469047e31ba23a003

What happens is that when MobArena gets an entity explode event, it removes the entity that caused the explosion from its own monster manager. This was perfectly fine before, because entities that cause explosions are very likely dead when they explode (i.e. creepers). The change to Obsidian Bomb makes it seem like the boss itself explodes. This is good for the monster-infight flag, because bosses then can't hurt their fellow aggressors, but it introduces this particular bug, so we obviously have to do something about it. I see a couple of possible solutions:

1. We revert the change to Obsidian Bomb, effectively making its explosions affect other mobs regardless of the monster-infight flag again. This isn't particularly desirable, but it is very easy to do, and frankly, the previous behavior is obviously miles better than this bug.
2. We find a different way to "clear" creepers, exploding sheep, and what else might explode and die in the process. I just spent a bit of time trying to get this to work in the most "obvious" way, but creepers don't actually trigger an entity death event when they explode, so the normal "clear mobs on death" procedure doesn't run for them. This means that MobArena continues to track them as if they aren't dead yet, but will eventually clear them when the wonky spawn thread timer rolls back around (every wave-interval and every 3 seconds after the interval if some of the clear-wave-before-* flags are set to true). It's technically not an issue as MobArena does clean up after itself at the very end of the session anyway, but it doesn't sit right with me.

An overarching problem here is that we generally do want to be reactive about monsters dying, because that's what will eventually lead to a much more reactive and robust session procedure, so removing monsters as soon as they explode does make a lot of sense, but only if they actually die. Creepers do, and exploding sheep are actually manually removed by the sheep bouncing procedure anyway, so unless I'm missing something, there aren't any entities in the game that explode without dying? Ghasts shoot projectiles, but I don't actually know if their explosions ignore the monster-infight flag.

If anyone wants to try out a build where I've removed the removal of entities that explode, hit me up on Discord. From my testing, it does fix the Obsidian Bomb problem, and it doesn't introduce any real problems. The "mobs alive" counter in MobArenaPlaceholders will probably still count exploded creepers as "alive", although I haven't tested that out yet.

[NightmareFox]

This was one heck of an issue to track down, holy crap...

It turns out this is a regression introduced in 0.108 by this commit: 4f78936

What happens is that when MobArena gets an entity explode event, it removes the entity that caused the explosion from its own monster manager. This was perfectly fine before, because entities that cause explosions are very likely dead when they explode (i.e. creepers). The change to Obsidian Bomb makes it seem like the boss itself explodes. This is good for the monster-infight flag, because bosses then can't hurt their fellow aggressors, but it introduces this particular bug, so we obviously have to do something about it. I see a couple of possible solutions:

1. We revert the change to Obsidian Bomb, effectively making its explosions affect other mobs regardless of the monster-infight flag again. This isn't particularly desirable, but it is very easy to do, and frankly, the previous behavior is obviously miles better than this bug.
2. We find a different way to "clear" creepers, exploding sheep, and what else might explode and die in the process. I just spent a bit of time trying to get this to work in the most "obvious" way, but creepers don't actually trigger an entity death event when they explode, so the normal "clear mobs on death" procedure doesn't run for them. This means that MobArena continues to track them as if they aren't dead yet, but will eventually clear them when the wonky spawn thread timer rolls back around (every wave-interval and every 3 seconds after the interval if some of the clear-wave-before-* flags are set to true). It's technically not an issue as MobArena does clean up after itself at the very end of the session anyway, but it doesn't sit right with me.

An overarching problem here is that we generally do want to be reactive about monsters dying, because that's what will eventually lead to a much more reactive and robust session procedure, so removing monsters as soon as they explode does make a lot of sense, but only if they actually die. Creepers do, and exploding sheep are actually manually removed by the sheep bouncing procedure anyway, so unless I'm missing something, there aren't any entities in the game that explode without dying? Ghasts shoot projectiles, but I don't actually know if their explosions ignore the monster-infight flag.

If anyone wants to try out a build where I've removed the removal of entities that explode, hit me up on Discord. From my testing, it does fix the Obsidian Bomb problem, and it doesn't introduce any real problems. The "mobs alive" counter in MobArenaPlaceholders will probably still count exploded creepers as "alive", although I haven't tested that out yet.

Maybe there is a way to fix it without getting other issues. Maybe when the obsidian bomb method is called it can be assigned a custom and/or unique identifier, I assume obsidian bomb runs on a timer before it explodes so then the I'd can get cleared after.

[garbagemule]

Maybe when the obsidian bomb method is called it can be assigned a custom and/or unique identifier, I assume obsidian bomb runs on a timer before it explodes so then the I'd can get cleared after.

The "physical" bomb is just an obsidian block, and it doesn't actually have any connection to the logic that deals with the explosion. Removing the physical block from the equation would leave us at the same spot we're in now, which also means that adding anything to the physical block doesn't provide us with anything useful either.

The "logical" bomb is just an explosion created at a location with a specific strength, and since 0.108, also a "source entity", which is the boss entity. But the explosion itself doesn't manifest as an entity or anything tangible, it just results in an event that MobArena then captures in the same way all other entity explosion events are captured. At that point in time, all we have to go by is the "source entity". We could check if the source entity is a boss, but a boss could also be a creeper, in which case there is no way (that I know of) for us to distinguish between the Obsidian Bomb explosion and the creeper explosion. The event doesn't carry enough information.

One way could be to keep track of entities that are "currently causing explosions without dying". Since Minecraft is single-threaded, and since the events are handled synchronously (when we trigger the explosion, all explosion event handlers are called before control is passed back to us), we might be able to distinguish between the two types of events that way. But we'd have to test that out.