multitheftauto / mtasa-blue

Multi Theft Auto is a game engine that incorporates an extendable network play element into a proprietary commercial single-player game.
https://multitheftauto.com
GNU General Public License v3.0
1.42k stars 438 forks source link

triggerClientEvent/triggerServerEvent not sending absolute number values #2361

Open ImSkully opened 3 years ago

ImSkully commented 3 years ago

Describe the bug With either triggerClientEvent or triggerServerEvent triggered and numeric values are passed with a decimal, the values that are received on the other end are only accurate to a certain margin.

To reproduce Client Code

addEvent("debugEventClient", true)
addEventHandler("debugEventClient", root, function(...) print("[CLIENT RECEIVED]", ...) end)

addCommandHandler("csend", function()
    print("[CLIENT -> SERVER]", 1, 1.2, 26.8, 1550.14, 1.111, 51.24, 0.0012)
    triggerServerEvent("debugEventServer", localPlayer, 1, 1.2, 26.8, 1550.14, 1.111, 51.24, 0.0012)
end)

Server Code

addCommandHandler("send", function(thePlayer)
    print("[SERVER -> CLIENT]", 1, 1.2, 26.8, 1550.14, 1.111, 51.24, 0.0012)
    triggerClientEvent(thePlayer, "debugEventClient", thePlayer, 1, 1.2, 26.8, 1550.14, 1.111, 51.24, 0.0012)
end)

addEvent("debugEventServer", true)
addEventHandler("debugEventServer", root, function(...) print("[SERVER RECEIVED]", ...) end)
  1. Use the command send to send values from the server to the client.
  2. Use the command csend to send values from the client to the server.

Expected behaviour The values passed should be equal to the actual numeric value which was originally sent, within execution the value sent should be fundamentally equal to the value received.

For instance if the client sends the number 26.8 to the server and a comparison is conducted, the value received may be 26.799999 instead, and the condition 26.8 == 26.799999 fails.

Screenshots img

Version

Additional context N/A

thisdp commented 3 years ago

this is normal to computer i think. 0.1+ 0.2 ≠ 0.3

dmi7ry commented 3 years ago

This is normal for floating-point arithmetic (google "comparing float points" for details). You should use epsilon to compare floats, like if math.abs(a - b) < 0.001 then. Also you may try (although I'm not sure would this always help) to send it as strings or integers (value = math.float(value * 1000) before send and value = value / 1000 for received result)(integers also can be send as strings, if need)

AlexTMjugador commented 3 years ago

IIRC there was an issue, maybe in the old bug tracker, about doubles being transferred in events between client and server without keeping all their precision bits (I vaguely remember that issue stating that only ~40 bits of precision were kept). So, even though due to how floating point numbers work some "seemingly easy" numbers can't be represented exactly, and exact equality comparisons are often wrong, I think that there is some undesirable precision loss going on.

After all, if you print exactly the same floating point number using exactly the same code, you should get exactly the same result, no matter if you are running on the client or not, or the number was transferred via an event.

Edit: if you search by "precision" in the old bug tracker, you can indeed find some issues related to what I was talking about: https://bugs.multitheftauto.com/view_all_bug_page.php?filter=61472db676d2c. @ccw808 commented in issue number 7389 that "precision loss is still there, but numbers (for element data and event arguments) are automatically rounded so no one will ever know", although I can't find what was changed in r6009 due to broken links.

Edit 2: a quick commit message search found this commit, which can be related to the issues at hand: https://github.com/multitheftauto/mtasa-blue/commit/042b39a6fdeddc16ad643914d6c5e9b4faa3a47f. In that commit is stated that the client runs floating point calculations with 53 bits of precision only, instead of the full Lua double 64 bits precision. This code wasn't fundamentally changed ever since: https://github.com/multitheftauto/mtasa-blue/blob/81bb2a7e1687b7c9efc4eff228629c87d7a187ba/Client/mods/deathmatch/logic/CClientGame.cpp#L6356-L6381

Edit 3: I wrongly mixed the "precision" and "format" bit lengths through this comment, but the point that some things are done counterintuitively in MTA's end still remains.

ffsPLASMA commented 3 years ago

So the question remains why does the client only use 53 bits of precision. This could be a serious issue if you rely on data exchange to be correct, wouldn't it be better to prevent such problems by rounding number before pushing them onto tasks like triggerEvent?

LosFaul commented 3 years ago

So the question remains why does the client only use 53 bits of precision.

local a = 2^53
local b = 2^53 + 1

print(a, b, a == b)
--> 9.007199254741e+15  9.007199254741e+15  true
AlexTMjugador commented 3 years ago

This could be a serious issue if you rely on data exchange to be correct, wouldn't it be better to prevent such problems by rounding number before pushing them onto tasks like triggerEvent?

I think they're rounded for transfer, but I wanted to point out that the whole situation is a bit more involved than it what seems in the first place, because there are lots of conversions involved and in some places the full 64-bit precision range is not used.

Pirulax commented 3 years ago

Works as expected IMHO. As you can see, double is only used in high value range, while in the lower range, float is used.

https://github.com/multitheftauto/mtasa-blue/blob/26565770db2662d84554d68e3680c0acbfc9eeeb/Server/mods/deathmatch/logic/lua/CLuaArgument.cpp#L613-L631

https://github.com/multitheftauto/mtasa-blue/blob/26565770db2662d84554d68e3680c0acbfc9eeeb/Shared/sdk/SharedUtil.Math.h#L80-L104

AlexTMjugador commented 3 years ago

Works as expected IMHO. As you can see, double is only used in high value range, while in the lower range, float is used.

Even though this behavior is expected due to the conversions that take place (double to float and float back to double, without actually using the full 64-bit precision in neither side), I find it somewhat counterintuitive. Sure, for the most part scripts can do just fine with numeric values being changed slightly, but if they are meant to transfer binary data, falling back to using a string to guarantee bit-for-bit identical delivery seems a bit wrong.

On the other hand, while I agree that saving bytes in network packets is important, I think that losslessly transferring numbers in events is important too, and there probably are better ways to achieve or even improve the compression that is currently obtained by lossy means. For example, the whole packet could be compressed (maybe this is done already, though). Or, by realizing that both client and server allegedly use 53-bit precision, 11 bits could be saved without any losses by not sending them and just setting them to zero on the receiving end.

Edit: my second idea is wrong due to ccw's comment below, as the full 64 bits of a IEEE-754 double precision floating point number representation are actually used with the 53-bit FPU precision control setting.

LosFaul commented 3 years ago

in general MTA should not convert doubles send in events to float at all as this is a source of bugs

ccw808 commented 3 years ago

_controlfp(_PC_53, MCW_PC); This sets 53 bits of precision in the significand, which together with the exponent uses 64 bits.

ImSkully commented 3 years ago

Even though this behavior is expected due to the conversions that take place (double to float and float back to double, without actually using the full 64-bit precision in neither side), I find it somewhat counterintuitive. Sure, for the most part scripts can do just fine with numeric values being changed slightly, but if they are meant to transfer binary data, falling back to using a string to guarantee bit-for-bit identical delivery seems a bit wrong.

Such a case like this is exactly what needs to be taken into consideration, lossless transfer between client and server is something that should be expected to be critically accurate.

Pirulax commented 3 years ago

Um.. binary data is just a string in Lua. What kind of binary data would you like to send as numbers? Also, since you want to send binary data, I'd assume you use integers, not floating point, in which case this precision loss isn't important.

ImSkully commented 3 years ago

I am not trying to send binary data across client or server, that was just an example provided by @AlexTMjugador and a good one at that; sending floating point numbers should be accurate to the decimal when received on the opposite end.

AlexTMjugador commented 3 years ago

After scratching my head about this for a bit, I can't really find a nice example that requires exact bit transfer for decimal numbers between client and server. I still think that doing so without at least documenting that in the wiki is a bad thing, though, but I don't have more compelling arguments than "it doesn't minimize API perplexity".

I considered transferring currency values between client and server precisely as a possible example, but if you really want precision for dealing with currencies, you should not be using floating point numbers at all, and instead use a fixed point notation based on integers, negating the shenanigans of floating point computations. In general, I agree with @Pirulax on his point that, if you really need precision, you probably should be using integers anyway.

But I think that the code responsible for converting numeric values for transfer looks kind of arbitrary in its logic. I mean, why any float up to 0x1000000 is considered to have an acceptable loss of precision? It can be argued that a ±1 error is quite significant, and such error could mess with further computations in a hard to debug way. For me it'd seem more reasonable to just check if the decimal value can be converted to an integer without losses; if not, then fall back to a float; and if a float is not lossless either, fall back to a double. But this is mostly based on opinion and not on real world use cases.

ccw808 commented 3 years ago

why any float up to 0x1000000 is considered to have an acceptable loss of precision?

The theory was that at values higher than 0x1000000, the float precision interval exceeded 1, so ints were better.

It should be noted that in the past the whole client operated in 32 bit float mode to accommodate directX, and much of this code was written on the basis that doubles would not give any extra precision

AlexTMjugador commented 3 years ago

That historical fact gives me the idea that we could switch back to that mode in both the client and server during Lua script execution, so both run with the same numerical precision, the bandwidth usage is not incremented at all, and everyone is happy, as most calculations I can imagine do not require the additional precision. Although I can see that change opening another can of worms, as anyone trying to store a full 32-bit integer in a Lua double will suffer because 32-bit floats can't accommodate the full 32-bit integer range with a precision interval of 1 or less. And the server has several threads AFAIK, but such a precision change is process wide.

I'm not sure what'd be the best way to tackle this, but in the meantime I can add a note to the wiki article of triggerClientEvent/triggerServerEvent linking to this issue and explaining the current state of things.