Stripping Down EntityDamageEvent - DamageModifier

Discussion in 'API Discussion' started by md_5, Nov 19, 2016.

?

Remove DamageModifier API?

  1. Yes

  2. No

Results are only viewable after voting.
  1. md_5

    Administrator Developer

    You can still check if the player is blocking when the event is called.

    I have made the assertion that the API is unmaintainable, and causes unavoidable inconsistencies with Vanilla and the majority have agreed with me.

    Unless someone completely rewrites it in a maintainable manner and fixes these bugs, it's going to be removed at some point in the future.
     
  2. But if I understand correctly damage is only reduced by blocking if you are hit head-on, on the sides and behind damage is not reduced. Without the API how would I check if the damage actually got reduced or not?
     
  3. I know, but like was said, this system just leads to more bugs and issues. Keeping it for the 1% that uses it wouldn't be very effective. Isn't there an attribute or sth that could disable the protection?
     
  4. That's only a small majority though (as in, there's still a 42% of the users who want to keep the API). IMO you should keep it till you can find a better system to replace it with, not scrap it and pray you will find a better replacement along the way.

    So instead of discussing whether to remove, might it be worth to start a thread on how to redesign the API instead?
     
    • Agree Agree x 3
  5. md_5

    Administrator Developer

    It's a pretty reasonable majority, especially when you consider most the people voting against it haven't given reasons.
    I assert it's not possible to have this API without rewriting large chunks of the damage handling, which is why this thread exists.
    It's a recent API and has, and continues to cause more issues than it has solved.

    "Keeping it in hope of a better system" is exactly why it wasn't removed months ago, but it's clear now that the only way we will ever fix some of these bugs is removing it, and I, and the community, think that is an acceptable approach.
     
    • Agree Agree x 1
  6. I'm changing my previous opinion on this. Its reasonable to remove API that is constant source of problems which is difficult to solve.
     
  7. A private plugin that I use and maintain that rewrites much of combat relies on this API. I ABSOLUTELY need to be able to detect if damage was reduced by blocking for some custom shield mechanics, and I also need to be able to have the damage from before any reductions occur (I use the BASE modifier) for some mechanics that work off of unmitigated damage.

    I do somewhat agree that it should probably be removed, it seems as if its only used in niche cases, but if I cannot get the above two details from other means, it will completely devastate core mechanics in the aforementioned plugin, which is used on my server as well as the 3 servers I provide it to. I won't be able to update for some time without them, so until I see a viable replacement for at least those two, its a no from me.

    Code (Text):
     
            // I guess this will work...
            double baseDamage = event.getDamage(EntityDamageEvent.DamageModifier.BASE);

            if (event.isApplicable(EntityDamageEvent.DamageModifier.BLOCKING)) {
                if (event.getDamage(EntityDamageEvent.DamageModifier.BLOCKING) != 0) {
                    isBlocked = true;
                }
            }
     
    EDIT: Even my own comments disapprove of this API, but it remains a necessary evil for me. Give us even a check for if the attack was successfully blocked, and I'll be satisfied.

    Are you fucking kidding me? o_O
     
    #67 Faceguy, May 15, 2017
    Last edited: May 15, 2017
    • Agree Agree x 1
  8. Maybe you'd get more contributors to help with things like this if you would take care of the growing list of PRs that have been racking up for the last 2 and a half years and stop declining and remaking PRs yourself? I stopped contributing due to reasons like this, so have many others. Currently, even if someone was to rewrite it or make a better API it would just sit there collecting dust until it is no longer merge-able. Just saying.
     
    • Agree Agree x 7
    • Winner Winner x 2
  9. I use this API to make protection enchantments not work, so that god gear (which everyone on my server *hates*) can easily be replaced with custom power armor. Currently, I can simply do this:

    Code (Text):
        @EventHandler
        public void onDamage(EntityDamageEvent event) {
            if (event.isApplicable(EntityDamageEvent.DamageModifier.MAGIC))
                event.setDamage(EntityDamageEvent.DamageModifier.MAGIC, event.getDamage());
        }
    I really don't want to make something larger than my entire current plugin just to change how much of the damage is affected by protection. Unless there's an equally easy way to do this, I definitely vote no.
     
  10. I can understand why md_5 would not want to do this. If the PR doesn't follow good coding practices, or doesn't match the general coding practices of his codebase, he really doesn't need to use it. I know I wouldn't want my codebase to be a huge mishmash of random community code, which would end up being very messy and such.

    EDIT: At the same time, I do agree in that I've seen a few good PR's that get denied, although those were actually for Bukkit, I dunno much about the ones for Spigot.
     
  11. I wouldn't really consider 60% a reasonable majority, 40% of people using it is still quite a bit...It's not like ebean, which was pretty much just zPermissions and private plugins like one of mine. But then, not sure if it's actually 40%...
     
  12. Some of those votes are possibly random or a part that's used that can easily replaced. There have only been few people who gave reasons for their choice, so it's difficulty to say whether it's really a 40% that's actively using the API.

    You could have clicked the edit button, ya know?
     
  13. It was a response to a different post though
     
  14. You can reply to multiple people
     
  15. Is it correctly understood that there are now no non-deprecated constructors to EntityDamageEvent?
     
  16. i wonder if we could have at least a isBlocked() method to check if the entity (which should be a player or a shulker) successfully used the shield reduction for the damage

    we can calculate anything else with a bit of math
     
  17. Problem is, what if I want to disable blocking damage, and so does your plugin. How do you expect we handle that?
     
    • Like Like x 1
    • Agree Agree x 1
  18. It's hard to make a RPG server without this. What was the last version for the EntityDamageEvent API?
     
    • Agree Agree x 1
  19. blocking reduce damage by 80% (if im not wrong), if you want to disable blocking reduction damage you know that the final damage is 20% so you multiply it by 5 to get back to original damage

    obiviusly its a bit more complex cause you have to consider the entitydamage cause and maybe some more minor details, but thats not impossible to do

    up to 1.11, you can still use it on 1.12 as deprecated methods but you may have some unexpected results
     
    • Informative Informative x 1
  20. but how do you know if another plugin has removed/added damage, and to which components?

    There's no way to know without Spigot keeping track of changes in the event object
     
    • Like Like x 1
    • Agree Agree x 1