NPC deals far too much damage

[Blamo27]

Hello,

As you can see on the video below, the NPC (all the sentinels in general, actually) seems to deal A LOT more damage than it is supposed to. Unless we don't really get how the damage system works compared to the one there was on Sentry, a damage of 18 shouldn't take more than 4 hearts to a player fully equipped with a Protection 4 diamond armor. In fact, previously it was equal to about 0.75 heart, so far less.

https://streamable.com/yji98

Also, for testing purposes we put something up to display to the player a message each times he receives damages. That message is "Dur chestplate: X". As you can see, the message displays twice every time the sentinel hits the player, which would imply the sentinel deals twice the damage (previously, with Sentry I mean, there was only one message displayed each time a Sentry attacked). So I think there is a double problem here : not only the Sentinel deals more damage that it is supposed to, but this damage is also inflicted twice (it can't be only that last thing, since a damage of 18 normally takes less than one heart, so the double shouldn't take four hearts).

[mcmonkey4eva]

What's your Sentinel config?

[Blamo27]

http://pastebin.com/0cKduHrb

[mcmonkey4eva]

The relevant damage code is more or less just a Bukkit call: LivingEntity#Damage(double, Entity) Do note that 18 damage, as far as Minecraft, is /a lot/. That's at the least as much as a strongly enchanted diamond sword. It's possible Sentry weakened the damage it output for some reason...

http://minecraft.gamepedia.com/Sword a diamond sword by default does 7 damage. You've nearly tripled that!

I glanced at the old Sentry code and can't even figure out where it applies damage at all so I dunno if that is what happened. It does appear to force-set damage event damage to the strength value, but beyond that I don't know what it's even... doing.

[Blamo27]

Actually, it appears that a 18 damage value is exactly equivalent to a sharpness 21 sword. So, we enchanted a sword with sharpness 21 and tried it repeatedly on a player equipped with a full Protection 4 diamond armor. The result is exactly as it were before : a little less than one heart damage (to be exact, it deals 1.963 damage, so roughly 98% of one heart). We did some more tests and try to find what Sharpness value it takes to deal the kind of damage the NPC dealt. The result is baffling : in one shot, the NPC takes 8.443 heart away, which is the roughly the same as a sword enchanted Sharpness... 83 ! So yeah, even by Minecraft standards, 18 should be far, far, far from this kind of damage. Again, it's a player equipped with a full Protection 4 diamond armor we're talking about !

Also, the fact that the damage event seems to happen twice is certainly not normal !

[mcmonkey4eva]

Huh. 8.44 is about 9 hearts... which is about 18 damage. Hah. Hahah. Uh. I wouldn't know offhand why armor just casually doesn't count anymore tbh. Again: I'm literally calling proper bukkit damage method. Everything that happens beyond that is Bukkit and minecraft's fault!

I'll look into alternative methods of apply damage. Though, one is already available: The workaround damage config option. Perhaps that'll yield ... different results?

[mcmonkey4eva]

asgdklsagk;lgsa I just looked and it's caused by enforce damage, which you have enabled >:(

Enforce damage goes past the default minecraft damage calcs to /enforce the damage value/

[Blamo27]

Well, obviously the 8.44 damage equals to 4.22 hearts, as shown on the video ! Absolutely not related to 18 damage x)

As for the enforce damage, of course we enabled it, you created it specifically to deal with one of our other issues ! https://github.com/mcmonkey4eva/Sentinel/issues/95

We can't disable it without having this other issue appearing again !

[mcmonkey4eva]

Well, obviously the 8.44 damage equals to 4.22 hearts, as shown on the video ! Absolutely not related to 18 damage x) you said 8.44 hearts not 8.44 damage. D:

[mcmonkey4eva]

AAaannnnd I don't know what to do then... short of internally calculating all damage modifiers (armor, etc) my self...

Actually, I have an idea, maybe Spigot will do something properly for once...

[mcmonkey4eva]

No promises but new build might fix it.

[Blamo27]

You're right, I meant 8.44 hearts in the way that players have 20 hp. It actually takes 8.44 out of 20 hp to the player.

[Blamo27]

So, latest build improved things : the damage is now far more reasonable, but is still doubled : by that, I mean it's twice the damage it should be, and the damage event appears to occur twice, as our displayed damage message indicates.

In other words, I think you fixed the first issue I mentionned in my post, but the second one is still there !

[mcmonkey4eva]

There's no reason I can see within Sentinel code that it should double - it might be Bukkit is doubling it...?

[Blamo27]

You may check the instances of your plugin, is the EntityDamage event set with the right priority ?

[mcmonkey4eva]

Why would that change anything 0.o

[Blamo27]

Did you test the event by debugging it ? Make sure you are instantiating the class only once.

I had exactly the same issue on a plugin of mine, but I don't remember why or how...

[Blamo27]

@mcmonkey4eva ?

[mcmonkey4eva]

After testing replication:

This only happens when you enable enforce damage but not workaround damage. So just flick the workaround config option on and it should be fine.

The basics of why: The regular damage goes through with complete disregard for the event being cancelled for some reason. So both the regular damage and the enforced damage apply, causing an exact doubling of damage. I Suspect a Spigot-side screwup, but don't feel much like bugging them about it.

[Blamo27]

Well this is annoying : when we switch workaround damage to true, the damages are indeed not doubled anymore... But they are once again far too high ! In other words, the issue we fixed earlier in this topic comes back when we enable workaround damage, which means the NPC deals much more damage than what it should deals, but they are dealt only one time, thus fixing the second issue. And if we disable the workaround damage, the "too much damage dealt" issue disappears, but the "damage inflicted twice" comes back. Seems like a dead end... What can we do ?

[mcmonkey4eva]

I don't even know tbh.

[AdraliK]

Good afternoon. I encountered this problem on my server, but it's a bit different. When I put 15 damage from a NPS with a "player" type, then I lose 2.5 hearts (in diamond armor) when I hit. If I change to a "zombie" type or another hostile mob with the same parameters, I lose 4 hearts . This bug was in all versions of this plugin. I ask that you would fix it in the next update or tell me how to fix it?

[Blamo27]

@mcmonkey4eva We're still having this issue ! Just in case, have you tried debugging the tryAttack method in the SentinelTrait.java file, to check if it was fired twice when workaround damage is set to false ?

[mcmonkey4eva]

I can't see anything happening wrong logically in current version (DEV-132) as best I can tell, workaround damage is doing the correct damage values, enforce damage is doing double damage for zero visible reason (I debugged the exact values my plugin sent out, but my hearts went down double).

Feel free to look at the source code here on github if you think you can spot something I missed.

Either way, try DEV-132+ from http://ci.citizensnpcs.co/job/Sentinel/ and enable /sentinel debug it will fill your console with what the NPCs are doing. Should something look wrong... post it here. If nothing looks wrong, but you still get wrong values in both modes... Well, I'm open to suggestions? lol

[Blamo27]

Sentinel debug did not provide any useful information, the tryAttack seems to occur only once, but the damages are in fact inflicted twice.

Just to clear things up, here is a recap of the issue :

• Workaround Damage = false ; enforce damage = true -> Normal damages, but inflicted twice each hit (sentinel with damage value of 18 = 1.963 HP taken away each hit, but since there are two hits in one, it's actually 3.926 HP taken away each time the sentinel strikes).
• Workaround Damage = true ; enforce damage = true -> Damages inflicted only once, but abnormally high (sentinel with damage value of 18 = 8.443 HP taken away in one hit).
• Workaround Damage = false ; enforce damage = false -> Damages inflicted only once, with perfectly normal values (sentinel with damage value of 18 = 1.963 HP taken away in one hit, which is exactly what it should be), but archers can't hit players in areas that block PVP (our whole Roleplay world).

In each possibility there is an issue we can't solve on our own, which seveverly affect the gameplay and the way the plugin is supposed to work. Fixing only one of this three issues would solve the problem. For instance, if the damages weren't abnormally high while using workaround damage, we could use it to solve the "damages inflicted twice" bug. Or, if the archers could fire at people in pvp-restricted areas, we wouldn't have to use enforce damage, thus avoiding the "damages inflicted twice" bug. Fixing the said bug on the enforce damage option would also solve the issue since the damage values are normal with it, just doubled.

I'm going to have a look at your code but if you could check out if any solution comes to your mind after this recap, that would be great of course.

[mcmonkey4eva]

Does the log show damage being inflicted twice? I logged where it applied damage in the debug system...

[Blamo27]

(I edited my previous message, i forgot to post a part of what I wrote)

Here is the log of the sentinel debug : as you can see, it first detects the player, goes toward him, attack him, then the player leaves and so does the sentinel. The tryAttack passed only one time but the damages were inflicted twice. https://pastebin.com/1kbLFXhk

[mcmonkey4eva]

... Wait, are you using archers? I've been testing with fist-punching... >.>

[Ferocimo]

Hello, I'm co-founder of the server of Blamo27. I will answer when he can't.

No, this test was performed on a standard sentinel wielding a diamond sword.

[mcmonkey4eva]

I don't even see it calculating damage values in the debug log... As best I'm aware, bar me missing something in the code, it should post damage tracking to the log...

[Ferocimo]

Well I don't see what I can do, I enabled sentinel debug on the sentinel and this showed up in the logs, nothing more... Do you think it might be because the NPC is wielding a sword and not using its own fists ?

[mcmonkey4eva]

Try build DEV-138+ off http://ci.citizensnpcs.co/job/Sentinel/ There should be a better quality debug log hopefully... Post whatever that outputs if you can? Should help track where damage is happening...

[Ferocimo]

I think you uploaded the wrong version : we downloaded the version you just sent, but it appears it's the build 131. We tried anyway with the debug but there was no changes at all in the log (same log as what we sent earlier).

[mcmonkey4eva]

... Mate. That's an automated build server. It does not mess up uploading files. You messed up the downloading of it.

[mcmonkey4eva]

Wait, you were on 131? You never even ran 132 as I asked?! D:

[Blamo27]

No no we were testing the latest version !

[Blamo27]

Firstly, you can't put Bukkit events into SentinelTrait Class, because it will fire events a lot ... You need to create a separate Class returning an instance of SentinelPlugin.

[mcmonkey4eva]

... what? That's intentional placement for careful logical reasons...

[mcmonkey4eva]

Also, I'm still awaiting an updated log from DEV-138+

[Blamo27]

So if you can't move your events out of SentinelTrait Class you should check the number of instances of it.

[mcmonkey4eva]

Those events output debug log info if they do anything. Thus the request for debug logs...

[Blamo27]

This is the debug log : https://pastebin.com/muWKvkza

[mcmonkey4eva]

I see it striking once, not twice, in that log...

[Ferocimo]

Yes that's what we said. The log doesn't show anything relevant, but the damage is indeed inflicted twice, there's no doubt. Our plugin clearly shows that the player is hit twice at the same time, and takes twice the damage. That only happens when enforce damage is set to true and workaround damage set to false, otherwise the damage is inflicted only once.

[mcmonkey4eva]

Your plugin helpfully stated 29.08 21:38:04 [Server] INFO [Broadcast] dmg : 2.462400436401367 once, not twice :P (I caught it before you edited the paste lol)

[Ferocimo]

This is not our plugin, that's actually a broadcast we added in your plugin, we removed it from the pastebin precisely to avoid any confusion.

Our plugin shows the damage being inflicted twice directly in the tchat of the player being hit.

[mcmonkey4eva]

Are you sure it's your not plugin reading it twice by mistake? Cause Sentinel is showing it being applied once pretty clearly...

[Ferocimo]

As you can see in this video we made when we created this topic, our plugin shows the damage being inflicted twice, with the message "dur chestplate: [x]" displayed twice per hit. https://streamable.com/yji98

Yes we're sure it's not our plugin reading it twice because the damage are indeed twice as high as what they should be, and what they are when enforce damage is disabled.

[mcmonkey4eva]

Can I see source code of your plugin?

[Ferocimo]

Blamo doesn't have it where he is right now, however that doesn't really matter. If you want, we can show you a video making a comparison between when we enable enforce damage and when we disable it. When it is disabled, the "dur chestplate" message is only displayed once and the damage is twice as low.

The fact that the damage is doubled is really indisputable.

[mcmonkey4eva]

No. Get source code. Cause I'm betting y'all forget the isCancelled check in the event. And thus are posting messages from cancelled events alongside actual ones.