oscar-broman / samp-weapon-config

A more consistent and responsive damage system with many new features
Apache License 2.0
93 stars 83 forks source link

Probably wrong variable in damage feed (needs to be tested) #250

Closed NexiusTailer closed 1 year ago

NexiusTailer commented 1 year ago

Recently I noticed that s_DamageFeedHitsGiven is used to get the issuerid's health inside damage feed taken message:

https://github.com/oscar-broman/samp-weapon-config/blob/7d41f6945d3bd5d9997f554f7a954ea79766aae0/weapon-config.inc#L5601

Seems like it was confused with s_DamageFeedHitsTaken array which is used everywhere else inside this condition except this single place: image

I need someone who have time now to test it and confirm that when you got some damage from the other player, your damage feed tells you wrong health amount in (brackets). If this is really a mistake, I'll change this to s_DamageFeedHitsTaken if it's needed.

ADRFranklin commented 1 year ago

I have been using this for sometime on my server now, and everything seems fine for me.

NexiusTailer commented 1 year ago

I.e. you get displayed your health lost (which indicated in brackets), not a health of the person who gave you damage, right?

UPD: could you also check any diff if this piece of code:

            format(
                buf,
                sizeof(buf),
                "%s%s - %s -%.2f (%.2f)~n~",
                buf,
                s_DamageFeedHitsTaken[playerid][i][e_Name],
                weapon,
                s_DamageFeedHitsTaken[playerid][i][e_Amount] + 0.009,
                s_PlayerHealth[s_DamageFeedHitsGiven[playerid][i][e_Issuer]] // <-
            );

will be changed to this?

            format(
                buf,
                sizeof(buf),
                "%s%s - %s -%.2f (%.2f)~n~",
                buf,
                s_DamageFeedHitsTaken[playerid][i][e_Name],
                weapon,
                s_DamageFeedHitsTaken[playerid][i][e_Amount] + 0.009,
                s_PlayerHealth[s_DamageFeedHitsTaken[playerid][i][e_Issuer]] // <-
            );

Unfortunately I still don't have enough time to finally check it myself in-game :(

ADRFranklin commented 1 year ago

Actually it doesn't change at all https://sharex.justmichael.xyz/KiqU4/zUYITUYo17.png

the health in the brackets is always 200.0 for me, despite being under 100 hp. (My server allows up to 200.0 hp in total)

ADRFranklin commented 1 year ago

nvm, it was just a small spectate issue, the health is of the offending player, so it's wrong still.

ADRFranklin commented 1 year ago

@NexiusTailer I'll try that code out and see what happens tomorrow.

NexiusTailer commented 1 year ago

So, does it need to be changed finally? I assume it is, but just to be sure it really shows relevant player's health after changing s_DamageFeedHitsGiven to s_DamageFeedHitsTaken in all tests.

ADRFranklin commented 1 year ago

I think both will work, but in both places you used the playerid even for HitsGiven, which is where I expect you got the wrong information. Instead would it have not made sense to use the attacking playerid for s_DamageFeedHitsGiven?

It's possible I misread what you put, so feel free to correct me.

NexiusTailer commented 1 year ago

So, yeah, I used playerid in HasGiven because this is how it's done now here: https://github.com/oscar-broman/samp-weapon-config/blob/master/weapon-config.inc#L5601

If both will work (using issuerid for HasGive variable or playerid for HasTake), then good, but I think it was just a typo mistake initially, somewhere in those PRs, and the author was probably meant exactly s_DamageFeedHitsTaken[playerid][i][e_Issuer] as he used this array data all around in this condition.

NexiusTailer commented 1 year ago

I should've tested it first before any assumptions as looks like this is my mistake though. Tested it with and without the Given -> Taken correction I made sure the current version works correctly with both given and taken damage names displayed, while replacing s_DamageFeedHitsGiven with Taken variable name here, messages about the taken damage sending wrong health of another player :/

Can be closed then, sorry for false find.