Bitfield REMOTE_STORAGE_PLATFORM_ALL changes value per platform

[TCROC]

Describe the bug
This was originally discovered in godot-rust/gdext when generating rust bindings for godotsteam: godot-rust/gdext#627. The issue is discussed in length in the linked PR. I'll highlight the key points here and questions we have.

Linux vs Windows

It was found that the enum REMOTE_STORAGE_PLATFORM_ALL has different values between Linux and Windows and appears to be treated as different data types.

file: isteamremotestorage.h

Platform	Hardcoded Value	Data Type	Value
Linux	0xffffffff	unsigned integer	4294967295
Windows	0xffffffff	signed integer	-1

This can be scene when hovering over the declared values in your IDE or stepping through in a debugger.

Linux:

Windows:

As pointed out by Bromean: godot-rust/gdext#627 (comment)

Semantically, bitfields are always unsigned, what do negative values even mean?

So that leads me to a question:

Is -1 the expected behavior for the bitflag on Windows?

If no, maybe a fix would be to explicitly set the enum's type to an unsigned integer here? https://github.com/CoaguCo-Industries/GodotSteam/blob/5348754e43e06ceae150888534047b64bbd6f5d2/godotsteam.h#L1473C8-L1473C29

Technically, Godot allows for negatives as seen here: https://github.com/godotengine/godot/blob/81f3d43cc1ba01136795fb2059bbaa55bc514a16/core/object/class_db.cpp#L949. The datatype for constants is int64_t which does allow for negative values. Hence we are going to update gdext to allow for negative values so that it compiles and is in line with Godot's data types.

We will be opening an issue on Godot's end as well to discuss whether we should force bitflags to be unsigned integers.

I look forward to any further clarification or discussing from the godotsteam team on this. Thanks again for the great asset! :)

[Gramps]

Hey there! Just finished reading through the original issue over on Godot-Rust's end and this reminds me of something I meant to check into. Possibly unrelated. During a local compile test in Linux, some of the (I believe) enums' values get swapped to -1 instead of what they should have been. I jotted it down in my notes and haven't had time to come back to that.

Originally, we used to assign the enums directly but then changed it to passing Steam's enums values after Valve changed a few of them. As I'm sure you've seen: REMOTE_STORAGE_PLATFORM_ALL = k_ERemoteStoragePlatformAll

I also recently noticed the generated docs give different values than I expected; again, on the list to check out but haven't investigated yet.

As for "Is -1 the expected behavior for the bitflag on Windows?", I am unsure. As I believe you mentioned in the Godot-Rust issue, this is a question for Valve. I'm trying to think of who is best to e-mail about these things.

After I get to my desk, I will run a fresh test compile and write down everything that get changed then add it in here as well as try to find any additional information I can.

[TCROC]

Thanks @Gramps ! I look forward to hearing what you find! :)

[Gramps]

Still digging around, went down a rabbit hole and learned about Two's complement today, so that was neat.

I believe there was another issue with types coming from C# so a much wider review on our end is probably needed; I'll flag that as a priority this week.

[TCROC]

Perfect! Thanks for the update! :) Haha Two's complement is always a fun one to learn about! :) Been a while so I'll do a little review with the link u posted ;)

[greenfox1505]

This boils down to the compiler treating enums as either unsigned or signed depending on the platform and resulting in different behaviors when that value is then cast to the 64-bit int that Godot uses. If we are seeing a different value in GDScript, this is why.

Rust caught this bug because it is especially explicit about how signed and unsigned values are cast. Whenever it notices an overflow, gives you explicit errors (except in release builds).

We don't have to be that strict. We can just force all these enums to be cast into signed 32-bit int before casting them to Godot ints. This will result in a more consistent behavior across platforms within GDscript.

[Bromeon]

We can just force all these enums to be cast into signed 32-bit int before casting them to Godot ints

For the case of 0xFFFFFFFF, this would however result in -1, which is what we'd like to avoid 🙂

I think int32_t is the problem in the first place -- if bitfields were initially cast to uint32_t on the other hand, that might work out better (assuming there are none which use more than 32 bits).

[Bromeon]

Has there been any decision on this issue yet?

Since Godot accepts int64_t values, I'd argue that any bitfield values that fit into uint32_t (a much smaller type) should be encoded as positive int64_t numbers.

[Gramps]

Not quite yet but that sounds like a good solution. I think the largest, off the top of my head, are probably results, maybe the networking configurations, and probably inputs?

I meant to get back to some of these issues before leaving for GDC but ran out of time. I'll chat a bit with @greenfox1505 when he gets here and get this resolved when I get back home. Sorry for the delay!

[Gramps]

OK, finally back from GDC and revived! I think casting them is where we settled... and as int64_t? @greenfox1505 maybe you remember better than I!

[TCROC]

OK, finally back from GDC and revived! I think casting them is where we settled... and as int64_t? @greenfox1505 maybe you remember better than I!

I believe int64_t yes. So that it matches the data type that godot uses. And if godot ever changes it to uint (which I believe there are discussions on, moving much more slowly) then we change to uint. But I'm a proponent of always matching godot's datatype.

[Gramps]

Sounds like a plan. I will get it rolled out in the next patch and, naturally, anyone else can PR it too.

[TCROC]

@Gramps And that was the discussion regarding the explicit data type. I guess another question, should bitfields be enforced to be positive? And how so? This was originally encountered due to how the platforms treated the values in the chart above.

[Gramps]

Hmm, probably? I don't know that any of them should be negative, right? I will have to do some more investigations, I think.

[TCROC]

I agree that they probably should not be negative

[Bromeon]

@Gramps keep the ongoing discussion on Godot side in mind: godotengine/godot#88962

There's no definitive decision yet, but it seems like the tendency is to move toward uint64_t rather than int64_t, as there's no good reason to support negative bitfield values. For now, the API still uses int64_t, so in any case it might be good to prevent negative values from being passed in.

[Gramps]

Thank you for the link; I went and read through it as well as the further linked stuff. Made some extra notes and sent my way-too-late-in-the-game message to the Steam team just to double-check their intended values.

[Gramps]

Well, I got a response from Valve but it didn't seem to clear much up. I did mention that the responses were different on different operating systems so not sure how to apply that to this answer. I did ask if the platform differences were intentional and what the expected value was supposed to be. Their reply:

Internally, we have this assigned like so for all platforms:
k_ERemoteStoragePlatformAll = 0xffffffff
I believe that if you are a 32-bit app, this would also be -1. And for a 64-bit app, that would return 4294967295.

While this information doesn't really help us, we will just move forward with the plan and just test!

[TCROC]

That's wild. That they would actually intend for -1 to be the value in certain cases... what do u think of this @Bromeon ?

[Gramps]

It doesn't really address the information you found though, right? Linux giving 4294967295 and Windows giving -1; this seems more like 32 vs 64-bit. To be fair, I'm not 100% how in-depth the knowledge of the person who responded was concerning the SDK itself or its inner workings. Perhaps they didn't understand what I was asking.

[TCROC]

This is a good point. Both of my builds were 64 bit and double precision. I expect the person responding may not have fully understood. Because I would be surprised to hear that -1 and 4294967295 is an intended variance. Even between 32 bit and 64 bit. I'm leaning towards there being some kind of misunderstanding.

In the mean time tho, I'm thinking i64 will solve our problems. At least on the short term.

[Bromeon]

That's wild. That they would actually intend for -1 to be the value in certain cases... what do u think of this @Bromeon ?

I still don't see any good argument for this discrepancy to be by design. Really, what is the advantage here?

If not causing problems with Godot constant registration, these values may be used in other places, like:

• hashing of objects
• serialization
• networking

Having two different values on Windows and Linux adds potential for errors and makes the above-mentioned functionality very hard to implement robustly.

I am not sure whether Valve was aware of such use cases in their reply; it's likely that their intention of flags is mostly to be directly used with bitwise operators in C or C++. Which works because it considers only the bit pattern, not the signed-ness. But I'd argue that even if they don't consider it an issue, that alone is not reason for us to propagate such errors to Godot.

---

In the mean time tho, I'm thinking i64 will solve our problems. At least on the short term.

I think that would work, but then cases like -1 would need to be handled explicitly, by mapping them to their positive equivalents (instead of directly casting to int64_t).

This approach would also be easy to migrate, if Godot decides to use uint64_t in the future.

[Gramps]

Remote Storage is a pretty old class in the SDK and I'd guess this is all primarily meant for C/C++ directly. Only recently was the Flat API added (last two years, I think) which was made to make bindings to other languages easier.

I'm a little curious to check out other SDK implementations, older ones, to see how they handle it. Like Steamworks.NET or maybe even Facepunch's. Well, especially Riley's Steamworks.NET since he contracted with Valve for a while.

That being said, moving forward with our assumptions and just testing the responses from Steamworks seems to be the best course of action. And easy enough to fix if the assumptions are wrong!

Update 1: Looking at Steamworks.NET, it seems to map 0xffffffff as -1. Unless I am misunderstanding something, which is always possible.

[TCROC]

Update 1: Looking at Steamworks.NET, it seems to map 0xffffffff as -1. Unless I am misunderstanding something, which is always possible.

Indeed it does! That's wild. Very strange indeed 😅

[Gramps]

So Valve does have a few enums that are negative values so I guess this isn't super surprising. In the case of Remote Storage platforms, I can see all being -1 if none is 0 and each platform is a positive integer. As all is never used with anything else, much like none.