Paper: Option to disable firework player damage.

[mibby]

Would it be possible to implement a Paper setting to disable player damage when in the vicinity of a firework going off? Quite a few plugins use fireworks for effects, resulting in players getting hurt now in 1.11+.

E.g.
McMMO leveling up / using abilities.

[MiniDigger]

the plugins that use fireworks should handle that themselves

[sgdc3]

+1 for this, it's a gameplay feature that should be configurable.

[zachbr]

... isn't this one of those things easily done in a tiny plugin?

[sgdc3]

Nope, the only way to disable the damage is very ugly:

package it.feargames.safefireworks;

import org.bukkit.entity.Entity;
import org.bukkit.entity.Firework;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.entity.EntityDamageEvent;
import org.bukkit.event.entity.FireworkExplodeEvent;
import org.bukkit.plugin.java.JavaPlugin;
import org.bukkit.scheduler.BukkitRunnable;

import java.util.HashSet;
import java.util.Set;

public final class SafeFireworks extends JavaPlugin implements Listener {

    private Set<Firework> fireworks;

    @Override
    public void onEnable() {
        fireworks = new HashSet<>();
        this.getServer().getPluginManager().registerEvents(this, this);
    }

    @EventHandler(ignoreCancelled = true)
    public void onExplosion(FireworkExplodeEvent event) {
        final Firework firework = event.getEntity();
        fireworks.add(firework);
        new BukkitRunnable() {
            @Override
            public void run() {
                fireworks.remove(firework);
            }
        }.runTaskLater(this, 5);
    }

    @EventHandler(ignoreCancelled = true)
    public void onDamage(EntityDamageEvent event) {
        for(Entity entity : event.getEntity().getNearbyEntities(5, 5, 5)) {
            if(!(entity instanceof Firework)) {
                continue;
            }
            if(fireworks.contains(entity)) {
                event.setCancelled(true);
                return;
            }
        }
    }
}

[zachbr]

Looks like there is currently a bug in the API that prevents someone from listening to EntityDamageByEntityEvent and easily cancelling it there.

That issue will be resolved, there will not be a configuration option added.

[sgdc3]

I still think it would make more sense to add an option, look at the other existing game mechanics related paper.yml entries.

[zachbr]

This is now working as of builds 1021+

@EventHandler
public void onEntityDamageByEntity(EntityDamageByEntityEvent event) {
    if (event.getDamager() instanceof Firework && event.getEntity() instanceof Player) {
        event.setCancelled(true);
    }
}

[sgdc3]

Oh thanks.... now spigotmc.org will be filled with 100 identical "plugins"

[zachbr]

Feel free to fork and do it yourself rather than just complaining. Or hell, copy and paste what I just wrote.
Why don't we add configuration options for everything else while we're here. I really hate how creepers blow up, I know a plugin could do it with less work and less maintenance burden but I really think we should bury it in a config.

Please.

[sgdc3]

Feel free to fork and do it yourself rather than just complaining.

What's the sense if you'll close the PR?

Or hell, copy and paste what I just wrote.

It was the fist version of code I tried to use.
Thank you for fixing the EntityDamageByEntityEvent, but i still think it should be a built-in feature.

I really hate how creepers blow up, I know a plugin could do it with less work and less maintenance burden but I really think we should bury it in a config.

"Paper is a high performance fork of the Spigot Minecraft Server that aims to fix gameplay and mechanics inconsistencies and improve performance."

[sgdc3]

# This is the main configuration file for Paper.
# As you can see, there's tons to configure. Some options may impact gameplay, so use
# with caution, and make sure you know what each option does before configuring.
# 
# If you need help with the configuration or have any questions related to Paper,
# join us in our IRC channel.
# 
# IRC: #paper @ irc.spi.gt ( http://irc.spi.gt/iris/?channels=paper )
# Wiki: https://paper.readthedocs.org/ 
# Paper Forums: https://aquifermc.org/ 

config-version: 12
settings:
  player-auto-save-rate: -1

enable-player-collisions: true

  bungee-online-mode: true
  load-permissions-yml-before-plugins: true
  save-empty-scoreboard-teams: false
  remove-invalid-statistics: false
  region-file-cache-size: 256
  incoming-packet-spam-threshold: 300
  sleep-between-chunk-saves: false
  min-chunk-load-threads: 2
  set-health-before-death-event: false
timings:
  enabled: true
  verbose: true
  server-name-privacy: false
  hidden-config-entries:
  - database
  - settings.bungeecord-addresses
  history-interval: 300
  history-length: 3600
messages:
  kick:
    flying-player: Flying is not enabled on this server
    flying-vehicle: Flying is not enabled on this server
world-settings:
  default:
    verbose: false
    keep-spawn-loaded-range: 8
    auto-save-interval: -1
    allow-non-player-entities-on-scoreboards: false
    hopper:
      push-based: false

**nether-ceiling-void-damage: false**
**enable-treasure-maps: true**
**treasure-maps-return-already-discovered: false**

    game-mechanics:
      disable-end-credits: false
      disable-chest-cat-detection: false
    container-update-tick-rate: 1
    remove-corrupt-tile-entities: false
    lava-flow-speed:
      normal: 30
      nether: 10
    use-chunk-inhabited-timer: true
    fishing-time-range:
      MinimumTicks: 100
      MaximumTicks: 600
    despawn-ranges:
      soft: 32
      hard: 128
    max-growth-height:
      cactus: 3
      reeds: 3
    falling-block-height-nerf: 0
    tnt-entity-height-nerf: 0
    fast-drain:
      lava: false
      water: false
    generator-settings:
      canyon: true
      caves: true
      dungeon: true
      fortress: true
      mineshaft: true
      monument: true
      stronghold: true
      temple: true
      village: true
      flat-bedrock: false
    non-player-arrow-despawn-rate: -1
    disable-explosion-knockback: false
    elytra-hit-wall-damage: true
    enable-old-tnt-cannon-behaviors: false
    use-alternate-fallingblock-onGround-detection: false
    grass-spread-tick-rate: 1
    portal-search-radius: 128
    squid-spawn-height:
      minimum: 45.0
      maximum: 63.0
    max-auto-save-chunks-per-tick: 24
    delay-chunk-unloads-by: 10s
    queue-light-updates: false
    use-vanilla-world-scoreboard-name-coloring: false
    optimize-explosions: false
    save-queue-limit-for-auto-save: 50
    water-over-lava-flow-speed: 5
    frosted-ice:
      enabled: true
      delay:
        min: 20
        max: 40
    lootables:
      auto-replenish: false
      restrict-player-reloot: true
      reset-seed-on-fill: true
      max-refills: -1
      refresh-min: 12h
      refresh-max: 2d
    filter-nbt-data-from-spawn-eggs-and-related: true
    keep-spawn-loaded: true
    armor-stands-do-collision-entity-lookups: true
    skeleton-horse-thunder-spawn-chance: 0.01
    disable-ice-and-snow: false

**fire-physics-event-for-redstone: false**

    baby-zombie-movement-speed: 0.5
    spawner-nerfed-mobs-should-jump: false
    allow-leashing-undead-horse: false
    all-chunks-are-slime-chunks: false
    mob-spawner-tick-rate: 1

**disable-thunder: false**

    disable-teleportation-suffocation-check: false

All features that can be implemented by a plugin "with less work and less maintenance".

P.S: Damn Markdown

[AlfieC]

sgdc3 I think you miss the point on those:

Teleportation suffocation check actually moves the "to" part so the player won't be teleported. That's default behaviour - and a plugin would not be able to retrieve the original location. That's why it can be disabled.

Physics event for redstone is a waste of CPU - how would not doing something be done with a plugin?

The thunder disable is for both practical and performance reasons.

[zachbr]

You aeem to miss the point of forking. The code is open source you are free to do what you want with it in any way with or without this project.

Adding toggles for cancelling damage negates the entire purpose of implementing a damage event API in the first place. This is an API implementation first, not toggle hell.

The point of "go fork it" was for you to run your own. Literally how this project was spawned.

[sgdc3]

Teleportation suffocation check actually moves the "to" part so the player won't be teleported. That's default behaviour - and a plugin would not be able to retrieve the original location. That's why it can be disabled

I know, i wasn't complaining about it, just read the options between the ** (should be bold but github markdown implementation has some issues)

Physics event for redstone is a waste of CPU - how would not doing something be done with a plugin?

we have access to the hanlder list of an event by using the getHandler() method

[sgdc3]

The point of "go fork it" was for you to run your own. Literally how this project was spawned.

The "go fork it" concept is completely useless if nobody is going to use your modified version.

[zachbr]

No it isn't. You're making a version for you with the features you want out of a piece of software.
It wasn't so long ago that PaperMC was in that same position.

If your argument is that our previous method of determining what is and isn't good for a configuration option is inconsistent, you may very well be correct. Adding more won't fix that issue. If you'd like to start switching more of those features to APIs rather than configuration options feel free.

But PaperMC does not exist to add a toggle to every single behavioral change upstream introduces, especially when it is already covered by a prominent API.

[aikar]

Many of the things mentioned that "could be done as a plugin" are just "Cleaner" in the server itself, and requires much messier plugin code to achieve it.

But this case..... is literally 3 lines and perfectly clean code that adds almost no overhead to the server.

And FYI:
**fire-physics-event-for-redstone: false**
No that can't be done by a plugin. The entire point of that change was to deprive plugins of ever even seeing it. To do it in a plugin completely invalidates the goal.

**enable-treasure-maps: true**
**treasure-maps-return-already-discovered: false**

Again no. No events exists for those.
Sure you could deny the result of the map but it would still be on the recipe list, which burns up Map ID's.

And the undiscovered only is hard coded in the server, almost impossible to do in plugin.

[mibby]

I love you all, please don't fight. ❤️ WorldGuard implemented some type of protection against firework damage using the other-explosion region flag. While not optimal, that will more than suffice for me (and hopefully others).

[sgdc3]

@mibby I don't want to fight, i see the point of @Zbob750 and @aikar but i don't completely agree with them, it's fine ;)