-
-
Notifications
You must be signed in to change notification settings - Fork 447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
setPedArmor and setElementHealth synchronization problems from Client to Server #3791
Comments
Health seems to be capped but yeah you can set any amount you want for armor clientside. So I can confirm this for the Armor. To be honest I think that via script you should be able to set any value for health and armor, it shouldn't break anything. We already have this for vehicles, why not peds? |
Tbh it should be fixed for all elements. You shouldnt have more max health just because. |
Yeh max health exists for a reason, it shouldn't be allowed to exceed it. |
Ok so, what's the actual reason? It works without issue with vehicles. |
My real problem is with server security, meaning preventing cheaters from adding more life than possible. It's 100 times safer to verify on serverside that the player has +100 armor* than on client and prevent it from being intercepted and cancelled. |
addEventHandler("onClientPlayerDamage", localPlayer, function()
cancelEvent()
end) You should really be doing serversided checks for this stuff by now (never trust the client, even if it has capped values). Also, what I am asking is to be able to set any health serverside, not even just clientside. Right now this is a bug, but it could be a feature and it would make it easier for scripters so they don't need to make their own health system when there is already one. Then again is up to the rest of us to decide if its worth it or not. |
It seems that he really doesn't know how to read what I wrote. |
Kinda rude but sure, since we are talking about capping values I think it's related. I will not take into account the unnecessary comments you made and I will just ask you what's the request? To cap the values clientside? If that's the case, then of course I would argue that maybe we should consider uncapping those values or making the cap higher. Why? So we don't have to rely on custom resources to handle another health system. Anyways the issue here is related to puresync most likely. What I think its happening for the server to correct their own values is that when the puresync packet arrives it already arrives with the values shorten down due to its maximum size, thats why the values are capped at 127.5 and 2047.5. So that's were the fix should be put in place, plus a cap in setArmor clientside (which is non-existent right now). Since we are already doing that we might as well add a cap for the other elemens in setElementHealth too. Now my argument, runcode So, if the server can set values higher than 2047.5 and it can also sync those values without much issue, why should we cap them low?
Could be, but why? The cap is not being done internally (there is a clamp in setPedArmor serverside but not clientside, but there is no clamp with setElementHealth in either client nor server except for ped/players). This is a side effect of puresync. If that's the case (game performing poorly) of course we don't want it, but if it isn't this could be a nice opportunity to fix this issue + raise the current limits of MTA. Or we can make our own implementation with lua and deal with it like we have been doing for years at this point... |
Cancelling event on the client is dangerous as the cheater can just not execute that code |
I'd like to point out something here because it has some connections with this issue. (kind of) A few years ago I was testing someting with clientside setElementData and things of like that and I also noticed this desynchronization thing. I wanted to make a custom hp+armor script and firstly I wanted to make it with setElementData (I dropped the whole idea after some days) What I want to ask is, why the maximum hp is 200 and not 150? In the singleplayer you can have a maximum of 150 hp. To get them you either need to do the paramedic side mission, or you'd need to exercise CJ's body to have more stamina+muslces to earn that slowly. This is a problem because the game HUD is designed for 150 hp and I'm surprised that no one caught this yet in these years. setPedStat(ped, 24, 1000) This makes your charachter to have MAX_HEALTH of 200. setElementHealth(ped, 100) Watch the HPBAR on the HUD. It'll look like its in the middle (almost) as it should. The other problem which also applies to this is the setPedStat itself which mislead the scripters. setPedStat(ped, 24, 1000) -- maxhp: 200 (so if 1000points means 200maxhp)
setPedStat(ped, 24, 500) -- maxhp: 84 (why 500points is not 100maxhp?)
setPedStat(ped, 24, 565) -- maxhp: 100 (in fact 565points is 100maxhp...) -- getPedStat returns 569 by default The best would be: setPedStat(ped, 24, 100) -- maxhp: 100 -- making this as default
setPedStat(ped, 24, 150) -- maxhp: 150
setPedStat(ped, 24, 200) -- error: maxhp can only be between 1-150 I'm only guessing but the reason that we can have 200max hp is because of the armor can not be set to 150. setPedStat(ped, 164, 1000) -- maxarmor: makes nothing, setting this stat doesn't do anything. In singleplayer we have max armor at 100, but if we do the vigilante mission then it will be increased to 150. In PC if I recall this feature is bugged and never worked in singleplayer as well. You couldn't have 150 armor even if you did the vigilante mission. (maybe a silentpatch could fix this but MTA doesn't (use/let use players to use) silentpatch. I understand that we couldn't have these unless we can use silentpatch, so at least the HUD bugs on hpbar should be fixed in the future and also the setPedStat should be reworked in my opinion. |
Who would have thought that after so many years I read something new about the difference in the GTA:SA ports. Good research. I guess the 84 life is done on purpose for when you don't take care of your character in the game. So that probably has to do with the 500 being 84 min health. Remember that in the game you can get "malnutrition" for not eating. I think that has something to do with this value. |
I think you misunderstood, setPedStat 24 is for setting MAX_HEALTH, that has nothing to do with starvation in the game. It's just the maximum health you can have, your current health is getting lowered by starvation and from damage. if you use setPedStat 24 with 500 value then your maxhp will be 84 which is lower than the default. It is a good thing if you want to make players to have way less maxhp by default. setPedStat(ped, 24, 1000) -- maxhp: 200 -- if 1000 for 200 then
setPedStat(ped, 24, 500) -- maxhp: 84 -- 500 should be for 100 not for 84
setPedStat(ped, 24, 569) -- maxhp: 100 -- this is the default now. |
Personally, I don't see any reason for functions like setPedArmor and setElementHealth to be available on the client side at all for non-local elements. What's the purpose? Why is it easier to use setElementHealth on the client side rather than on the server side? Most events like onClientPlayerDamage also have their equivalent on the server side. I understand the argument that removing these functions wouldn't change anything in terms of cheaters, because they can simply modify the value in memory, but in my opinion, it's still a big step forward. Let's not forget that most cheaters barely know how to start a computer, they use ready-made tools like lua executors, type in setElementHealth in some onClientRender and are literally invincible. Regarding the issue itself... I think it's perfectly fine to add safe value reduction when receiving synchronization packets on the server side. In that case, an example value like 2047 will only be seen by the client who executed setPedArmor with that value, while the server and other players will receive the 'corrected' value. As for the problem described by @derxgbb with MAX_HEALTH, it was explained in his issue. Regarding setPedStat, it's a matter of how these stats work in GTA. In San Andreas, it functions in such a weird and clunky way, and I honestly doubt anyone would be willing to overhaul the entire stat system to 'simplify' it for scripters. Also, let's not forget the legendary backward compatibility. The problem with MAX_ARMOR, I think we can fix it, even if the bug is in GTA's code, but we'll need more details for that, preferably a separate issue |
Thats because of puresync. If the client doesnt get damaged then it wont trigger the serversided event (even if it should). The server has no actual clue if a bullet hit, or if someone has fallen, because it has no idea of where the world elements actually are and has no access to the engine. I don't think this is feasible or even possible.
Yes and I don't get why we would want to limit scripters because of cheaters. Specially since this won't really fix the core issue (we need client's information to sync, MTA Server has no clue of the world). We as scripters just need to be clever about how we write our scripts. For example if we disable the ability to skip damage clientside then we would let our players die if they jump those buggy walls that make the game think you are falling at high fps. if weapon== 54 and getPedSimplestTask(localPlayer) == "TASK_SIMPLE_CLIMB" then
cancelEvent()
return
end |
I think you misunderstood me. I'm not talking about completely removing synchronization or events, as that would be absurd. Synchronization is necessary to send game state information to other clients via the server, and that's not up for debate. What I meant is that functions like setElementHealth should not be available on the client side for non-local elements. This would slightly increase security, as a simple Lua executor wouldn't be able to manipulate player health anymore, and more advanced memory modification techniques would be needed (MTA has kernel-level protections, so bypassing them is a complicated process). In that case, canceling the onClientPlayerDamage event would work as intended. For years, the setPedArmor function was server-only, and it should stay that way. Additionally, the same should apply to setElementHealth. That's my humble opinion |
Describe the bug
Creating anti-cheat checks for a server, we realized something curious.
Although the MTA wiki indicates that the vest that can be placed on a player is from 0-100, this is not real, you can set the number you want (example 1000) from the clientside, from the serverside this is not possible.
Testing this problem, I reached several conclusions.
Note:
Steps to reproduce
Version
Client: Multi Theft Auto v1.6-release-22763
Additional context
Relevant log output
No response
Security Policy
The text was updated successfully, but these errors were encountered: