minetest-mods / 3d_armor

Visible player armor & wielded items for minetest
Other
17 stars 40 forks source link

No armor damage from mobs #24

Open daretmavi opened 3 years ago

daretmavi commented 3 years ago

No damage to armor from monster attacs.

Test procedure:

It looks like there is a local variable "damage" set to false and the never come back to true, because mobs hat no groupcaps:

E.g. oerkki groupcaps:

{
    max_drop_level = 1,
    groupcaps = {

    },
    damage_groups = {
        fleshy = 4
    },
    full_punch_interval = 1,
    punch_attack_uses = 0
}

Temporary solution could be to set damage = true when no groupcaps defined?

--- a/3d_armor/api.lua
+++ b/3d_armor/api.lua
@@ -355,7 +355,10 @@ armor.punch = function(self, player, hitter, time_from_last_punch, tool_capabili
                                local level = damage_groups.level or 0
                                local groupcaps = tool_capabilities.groupcaps or {}
                                local uses = 0
-                               damage = false
+                               if #groupcaps > 0 then
+                                       damage = false
+                               end
+                               --damage = false
                                for group, caps in pairs(groupcaps) do
                                        local maxlevel = caps.maxlevel or 0
                                        local diff = maxlevel - level
sirrobzeroone commented 3 years ago

I did a fair bit of testing around this and can confirm the issue for the current versions of MTG+3d_armor and mobs_monsters. I also tried going back to old versions and for the life of me I couldnt get the armor to take damage all the way back to version MTG 0.4.17 and 3d_armor 0.4.12. of course I have no clue when the cool little armor damage bars were added. I don't know how many times I died with wood armor on different versions but pretty sure they should have broken. I'm in singleplayer I havent tried to test on a server but maybe the bug dosen't present for servers?

I did skim the code for 3d_armor versions 0.4.10 to 0.4.17 and the code block highlighted above for armor.punch changed in 0.4.10>>0.4.11 significantly, 0.4.11>>0.4.16 no code change, 0.4.17 minor addition of set_state and set_count

I'm very confused myself as Im pretty sure Ive had armor break before under old versions using mobs monsters but maybe im not remebering correctly, it would seem highly unlikely a bug like this has persisted since 2018-ish...

I can confirm the fix suggested by daretmavi - commenting out "damage = false" fix's the damage issue to armor. I completely agree with daretmavi that it is the groupcaps causing the issue as right before that code block damage is set to false then groupcaps is nil/empty so the loop never runs and hence damage never gets set to true again.

I did write this little code block which explcitly just allows the groupscaps==nil to cause damage but shouldn't allow any other cases to be set to true if there are any...I admit I dont understand the code enough, its possible setting to false there is uneeded but it looks very specific to me and like it is needed for something....

if next(groupcaps) == nil then --new line damage = true --new line else --new line
for group, caps in pairs(groupcaps) do local maxlevel = caps.maxlevel or 0 local diff = maxlevel - level if diff == 0 then diff = 1 end if diff > 0 and caps.times then local group_level = damage_groups[group] if group_level then local time = caps.times[group_level] if time then local dt = time_from_last_punch or 0 if dt > time / diff then if caps.uses then uses = caps.uses * math.pow(3, diff) end damage = true break end end end end end end --new line

I do find setting the above armor degrades very fast, how many hits should wood armor last? a fresh set at full health being attacked by a mobs spider lost about 15% just before death....I can probably go look the formula up in the api :).

I'll keep tryign to poking around :)

sirrobzeroone commented 3 years ago

I've been on the look out for other evidence this defect has been around for awhile and found the odd comment on the forum board and more importantly this on Stu's version:

https://github.com/stujones11/minetest-3d_armor/issues/177

cross linking here as it provides a 3rd confirmed report.

I'm still trying to workout why the default==false is there but given the importance of a fix is it worth me doing a pull request with those 4 lines of code I added as an initial workaround fix? or happy for me too keep looking or is someone more lua experienced also looking at this - I don't mind whichever :)

BuckarooBanzay commented 3 years ago

the changes proposed from @daretmavi look relatively sane :+1: , a PR would be nice even if it is just a (hopefully) temporary workaround

sirrobzeroone commented 3 years ago

Im working on the pull request using @daretmavi fix, Im just working through the code and Im not sure # is the bets to use as I'm pretty sure groupcaps table is structured as a non-numeric table, eg:

        groupcaps={
            fleshy={times={[1]=2.00, [2]=0.80, [3]=0.40}, uses=10, maxlevel=2},
            snappy={times={[2]=0.70, [3]=0.30}, uses=40, maxlevel=1},
            choppy={times={[3]=0.65}, uses=40, maxlevel=0}
        }

and #table_name in lua has problems counting non-numeric enteries in a table so Im a bit worried that potentially we could enter the groupcaps for loop below this with damage=true instead of damage=false that its expecting to start with. How about this as a fix I used @daretmavi idea just flipped it a little and used next?:

if next(groupcaps) == nil then
damage = true
end

Code would end up

--- a/3d_armor/api.lua
+++ b/3d_armor/api.lua
@@ -355,7 +355,10 @@ armor.punch = function(self, player, hitter, time_from_last_punch, tool_capabili
                                local level = damage_groups.level or 0
                                local groupcaps = tool_capabilities.groupcaps or {}
                                local uses = 0
                                damage = false
+                             if next(groupcaps) == nil then
+                                     damage = true
+                             end
                                for group, caps in pairs(groupcaps) do
                                        local maxlevel = caps.maxlevel or 0
                                        local diff = maxlevel - level

Im very very happy to take feedback I am not very experienced with lua I just fell over that #table thing once when I was doing something else, but if its not an issue happy to go with the orginal code suggested :)

daretmavi commented 3 years ago

Hi, I'm new in lua, so change it as you think is better. In Lua Reference I found this:

__len: the length (#) operation. If the object is not a string, Lua will try its metamethod. If there is a metamethod, Lua calls it with the object as argument, and the result of the call (always adjusted to one value) is the result of the operation. If there is no metamethod but the object is a table, then Lua uses the table length operation (see §3.4.7). Otherwise, Lua raises an error.

BuckarooBanzay commented 3 years ago

Im very very happy to take feedback I am not very experienced with lua I just fell over that #table thing once when I was doing something else, but if its not an issue happy to go with the orginal code suggested :)

Using next() for that sounds good IMO :+1:

sirrobzeroone commented 3 years ago

thanks both, I'll finish up some testing and then do a pull request if it all looks good.

@daretmavi if your interested in the weirdness of # push the below code into the https://www.lua.org/cgi-bin/demo site and run. My expectation was the same as yours from reading the lua doco both t1/t2 should =2 when using #. I got very frustrated nailing a bug down which turned out to be how # works in lua - I think of it now as useful only for numerical stuff but I know that's an over simplification :)

local t0 = {}
local t1 = {fleshy = {times={[1]=2.00}},snappy = {times={[1]=2.00}}}
local t2 = {{fleshy = {times={[1]=2.00}}},{snappy = {times={[1]=2.00}}}}

print ("empty total: "..#t0)
print ("word as key total: "..#t1)
print ("numerical key total: "..#t2)
sirrobzeroone commented 3 years ago

I did some testing checking damage before and after the fix and from what I can tell it dosent change the damage amount recieved to the player but does change the amount recieved to the armor - see attached screenshot_20201207_180114

I would have liked to have tested the groupcaps out further but I have no-one to test against as I would need two players for example as a test:

one wielding an axe hits the other player wearing wood chest plate and something would happen....Im not sure what maybe the axe would take less wear or possibly the damage the chestplate more or faster hit rate would do full damage. I cant tell without adding some output to that code block and actually testing it (or digging real deep into the code)....but I would expect a change.

I did try adding groupcaps to a mobs:redo spider but no luck with that. The more i look at the code the more I think it relates to the hitters weapon taking wear/uses.

Few things I noted while testing: Not sure if the below are intended behaviour but just seems odd to me - testing the behaviour below is the same with and without the fix so nothing new -

First thing I set a mobs:redo - spiders damage to 10 to test this

  1. Put on wooden chest plate (you can put only boots on same effect) - blocks 1 damage
  2. Put on full set wood + shield - blocks 1 damage

Try the same test with diamond

  1. Put on diamond chest plate (you can put only boots on same effect) - blocks 1 damage
  2. Put on full set diamond + shield - blocks 2 damage

I then did the same test with the spider at 5/1 damage and same amount of damage blocked ie 1 for wood 1-2 for diamond.

Personal view: More armor, block more damage - maybe adjustable by upping multiplier?

Second Thing Armor can fully block all damage - Set spider to 1 damage

  1. Put on Wood chest plate - Stand there getting zero damage on the plus side the armor does get damaged now so eventually armor degardes to nothing and breaks and then you start taking damage

Personal view: No matter how much of walking tin can you are there should always be a chance to take damage. Even if it's a random 1 in 3 you may take 1 damage.

I'm not sure if either of the above are what is expected? just noting it seemed a little odd to me. Ill admit I've never dug this deeply into 3d_armor before.

I'll do a pull request now as it all looks to be working okay to me.

Edit: Sorry about the closed request I got trailing whitespace after comment error so closed the first then I fixed up and then re-submitted.

Edit2: Sorry Missed this issue -

Issue 3 No matter how many bits of armor I wear they all take the same wear replicate:

Wear 1 wood chest plate it takes 2000 a hit

Wear 1 wood chest plate and 1 pair of boots they both take 2000 a hit.

Given Issue1 not sure why you would ever wear more than 1 piece of armor

Personal View: I'd spread the uses across the number of bits of armor. I know that gets complex when someone wears mixed armor but maybe somethign simple like If Im wearing 2 pieces:

Wood chest plate uses normally 2000 - 2000/2 = apply 1000 uses Diamond Helmet normally uses 200 - 200/2 - apply 100 uses.

Basically spread the damage around by dividing usage by num pieces worn

sirrobzeroone commented 3 years ago

Ignore Issue 1 and 2 above, I thought 3d_armor had more functionality than it does. Im guessing Stu must have had something planned but never implimented it :)

Issue 3 still a bit of a concern although I've been able to identify the code were thats assigned and have a wip fix/improvement

sirrobzeroone commented 3 years ago

Found this from stu on a mt engine thread

_ The 3d_armor mod was originally based on MTG and is mostly geared towards pvp since the game has no mobs by default. Though I did add level coefficients to help address this problem, it's still not a complete fix. stujones11/minetest-3d_armor#127

The problem I have, as others have pointed out, is the lack of precision and the reason I added the 'healing' properties to armor. @Ekdohibs has an interesting suggestion which is effectively the opposite of this and might be worth looking into.

Max health is now settable as a player object property.

This is indeed a step in the right direction, however, it needs to be implemented by the game. I do not think it would be right for the armor mod to interfere with this. IMO a resolution of 100 would suffice and it should be possible to depict the hearts in approximate tenths.

Wear only happens when a player gets fall damage.

By default armor takes wear according to the tooldef of the weapon used, it was intended to work much like digging the armor material. It is up to mob modders to pass a valid tooldef when punching the player.

I might add that armor also takes damage when the player is hurt for any other reason (such as falling). This is necessary to overcome the problem of mods that simply ignore the built-in damage mechanism and set player HPs directly, the tnt mod does this, irrc. _

sirrobzeroone commented 3 years ago

I"ll just summarize my thoughts below:

Short I'd go ahead with the fix as is for the moment, with idea to implement a better sol'n down the road - not sure what that might be see Long.

I think the application of wear to armor should be addressed but as a separate enhancement/change , given the unknown impact that may have on PVP combat damage to armor which i think 3d_armor is used for pretty broadly on servers and should currently be behaving correctly, I don't think risking stuffing that up is worth a hurried change.

Long I don't think it's practical at the moment to go and update all mob:mods although totally understand Stu's reasoning and agree with it. I do think armor should still take damage even if a mob is mis-configured and missing the data, in the same thread I lifted the above from someone mentioned armor not taking damage from mobs (circa 2017). I don't fully understand all the damage armor stuff yet even at base game level let alone adding 3d_armor over the top however one bit I do understand is tool "groupcaps" definitions which for tools is normally structured like:

groupcaps={
cracky = {times={[1]=4.00, [2]=1.60, [3]=0.80}, uses=20, maxlevel=2},
},

I'm not entirely sure first if its possible to store all those metrics against an entity, I assume they can be but it doesn't seem like a trivial update to me as it would mean changes to the entity template then changes to the damage handling code to ensure those new values are sent, worse still many mob frameworks are currently not being worked on - which would leave this as a persistent problem. On top of that "uses" would be applied to entity, as most entities ie spiders for example are using natural weapons that can't really be used up making uses a bit of a nil point....although maybe I'm overthinking it as 2-3 hits and they'll be dead anyways. Maybe uses can be set to 0 or nil and then they are ignored, possibly that could cause a failure then in the damage/use feedback? but that i don't know, just to many bits I don't know about. Given the uncertainty around the ability of an entity to hold all the required data metrics, let alone if damage/wear should be applied back on there natural weapons. Given those two biggish reasons I think the current fix of if groupcaps is nil/null apply damage is valid.....I'd love to get Stu's thoughts.

sirrobzeroone commented 3 years ago

I looked a tiny bit more at the issue of mobs having groups and capabilities. I did my looking at mobs:redo api, started with the nice simple damage stuff:

So all damage from mobs is assumed to be fleshy (thats physical attack only not shooting)
api line 2556 - damage_groups = {fleshy = self.damage}

The base template assumes a number for damage and has a difficulty multiplier applied to the damage api line 3487 - damage = max(0, (def.damage or 0) * difficulty),

So to have radioactive spiders dishing out damage not in the fleshy group is going to take a little bit of work, but looks like it could be achievable. I didnt dig very far at the moment just cursory look and putting this info here so I dont have to hunt all those lines up later if I want to dig a bit more.

mckaygerhard commented 2 years ago

hi i know that maybe cannot be significant enought but maybe the issue #57 has nothing to do with? it defines on_punched and means to set damage to true! @sirrobzeroone if that is not working.. damage never get true.. as i know event if that is own 3d_armor definition.. there's no such callback on minetest api