godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.79k stars 20.13k forks source link

Formatting options for integers #6949

Closed rvcalisto closed 5 years ago

rvcalisto commented 7 years ago

Lately when handling netcode on Godot, I've been using RawArray to send a customized packet array to my server by put_data(RawArray), and could only do it because the .to_utf8 converts every char from a string into a byte inside the array.

So when I was going to write other types of data into the packet array, I noticed that only strings have this formatting feature.

So my proposal is adding features like int.to_32, that would return a RawArray, just like .to_utf8 does. It would make the netcode less bound to Godot's packet specification.

Zylann commented 7 years ago

Isn't this doable with StreamPeerBuffer?

rvcalisto commented 7 years ago

Yes it is, I just saw your reply on it, thanks again. However .to_utf8 still exists, functions with the same capabilities, as .to_32 also should, don't you think?

Edit: Also, I tested it now, using .put_utf8_string on the StreamPeerBuffer uses the Godot's packet specification header. The point is making netcode in Godot less bound to it, for practicity sake.

Zylann commented 7 years ago

Are you sure StreamPeerBuffer::put_utf8_string() does that? From what I see in C++, it would just ends up with this structure in the raw array: <unsigned int><8-bit characters...>. Where did you see a Godot specification header? If there is one it doesn't seem to come from StreamPeerBuffer.

rvcalisto commented 7 years ago

Code: var stream = StreamPeerBuffer.new() stream.put_utf8_string('string') var raw = stream.get_data_array() client.put_data(raw)

Output (server-side): <Buffer 06 00 00 00 73 74 72 69 6e 67>

See the those 4 bytes in front of the actual string? They are the Godot's specification header.

Zylann commented 7 years ago

The four bytes you see are not Godot's specification header, they are the length of the string as an unsigned int, with a big-endian encoding, as I wrote in my last post. So... yeah, they are kinda like a header, but it's actually very common to send strings like that, it's not something Godot-specific tbh, but more like a convention choice. Also the big-endian thing is also very common because network often works that way, as far as I can tell.

Is your server actually relying on null-terminated strings? Maybe that's your actual problem?

rvcalisto commented 7 years ago

I know they're the length (yet traces of the header), and I know they are very common, but i don't want to rely on this predetermined feature as they are. I send my length in 8bits, and is the length of the full packet of strings and integers, that are, yes, terminated with null, each one in the array.

I just want the int.to_32, otherwise me and whoever wants to use custom data packets have no choice other restructuring the server code to accommodate Godot's predefined architecture.

vnen commented 7 years ago

I think the C++ code has some functions to do this. Maybe they could be added to the Marshalls singleton.

Zylann commented 7 years ago

Why do you want int .to_32? When using a StreamPeerBuffer, they end up as the same data, you know? (like, 4 bytes).

However I agree it's a bit annoying to not have a way to choose how more complex objects like strings are structured in the buffer, because conventions are legion.

rvcalisto commented 7 years ago

I guess I'm a failure trying to represent my specific problem to you @Zylann, it's not the first time you seem to not understand my reasoning. I won't save any lines this time.

packet desired (bytes): [0b 48 45 4c 4c 4f 00 01 00 00 00] The way I currently go around it now: I create a RawArray, then I .append_array the string 'HELLO'.to_utf8 to convert each char to byte inside an array. Then I append a single '0', this is converted by default as a null single byte. The next part is where I'm stuck, I want to send a 32bit integer, in this case is a simple '1', but it has to be 32bit, as it is with any integer handling in the server. But there is no way I can put this in, appending a '1' as I do with the '0' will turn it into a single byte, it needs to be 4. No escape.

(The first byte in the packet is my pkt.lenght, I put it before sending with array.insert(0, array.size()+1))

With the StreamPeerBuffer I can use .put_32, wich gives me the 4bytes array, however, my desired packet needs a string don't it? So before the integer, I use the string.put_utf8_string on the buffer. Surprise, it comes with 4 bytes of length I didn't ask for. Even tried.set_data_array() and put the string.to_utf8 to go around the 4 bonus bytes, it works, but as soon I try to put an 32bit int with .put_32 on the buffer, it writes above the existing bytes, as it can't append data. I'm blocked.

I hope this clarifies my situation for you.

Zylann commented 7 years ago

there is no way I can put this in, appending a '1' as I do with the '0' will turn it into a single byte, it needs to be 4

What? How? That sounds completely wrong (unless you actually write the quotes?).

I understand how unpractical it is currently, and in your case you would have to use StreamPeerBuffer anyways (and I would too, since I don't get why StreamPeerTCP sends everything per put_* calls, I was used to make buffered packets everytime before, using typical serialization).

StreamPeerBuffer works with an internal cursor pointing at the next address things will be written, like a file. When you use set_data_array(), it surprisingly won't move that pointer, that's why you end up overwriting your data. However, you can use the seek function to position the cursor after the bytes you added :)

But that will be even harder to work on if you need to put another RawArray afterwards, because you would have to use put_var, which I guess does again a formatting you don't want as well, so you would have to create a second StreamPeerBuffer which gives you another RawArray and... no, wait. Did the snake just ate his own tail here? :s

Maybe the title of this issue is not explicit enough, because the problem here is that serializing arbitrary data into a buffer with custom formatting looks surprisingly complicated in some cases (not only integers) when using only GDScript... that's such a low-level thing the API was never thought for it :|

rvcalisto commented 7 years ago

It is not wrong, try it for yourself, why do you think appending a integer to an RawArray would return anything other than a single byte?

I don't really need StreamPeerBuffer for sending strings and 8bit integers right now, I can do it with RawArray, .to_utf8 and .put_data(), just want the .to_32 to be able to go on using Godot for multiplayer.

Is very simple, really. If .to_utf8 exists, so should .to_32, .to_16, .to_8, give the tools for the people and let the heavy lifting for the coders that want customization, what is the point of .to_utf8 otherwise?

StreamPeerBuffer seems to work as intended, for people that like the way Godot's packet arquitecture currently works.

bojidar-bg commented 7 years ago

I agree with @Lohan120, .to_32 or RawArray::append_32() is going to be pretty helpful for hard binary work.

Zylann commented 7 years ago

But... I thought RawArray was just a way to pass data on the stack, and not a "stream"? If you go that way, we should add all append_xxx functions to it, then. Which leads me to question the relevance of StreamPeerBuffer :s

ghost commented 6 years ago

First of all thank you for your report and sorry for the delay.

We released Godot 3.0 in January 2018 after 18 months of work, fixing many old issues either directly, or by obsoleting/replacing the features they were referring to.

We still have hundreds of issues whose relevance/reproducibility needs to be checked against the current stable version, and that's where you can help us. Could you check if the issue that you described initially is still relevant/reproducible in Godot 3.0 or any newer version, and comment about it here?

For bug reports, please also make sure that the issue contains detailed steps to reproduce the bug and, if possible, a zipped project that can be used to reproduce it right away. This greatly speeds up debugging and bugfixing tasks for our contributors.

Our Bugsquad will review this issue more in-depth in 15 days, and potentially close it if its relevance could not be confirmed.

Thanks in advance.

Note: This message is being copy-pasted to many "stale" issues (90+ days without activity). It might happen that it is not meaningful for this specific issue or appears oblivious of the issue's context, if so please comment to notify the Bugsquad about it.

akien-mga commented 5 years ago

@bojidar-bg @godotengine/network What's the status here, is this still needed?

Faless commented 5 years ago

Fully customized binary serialization is possible via StreamPeerBuffer.

As an example:


func get_my_buffer():
    var buf = StreamPeerBuffer.new()
    buf.put_data("string".to_utf8()) # The string, without any godot serialization
    buf.put_u8(0) # The terminator you want
    buf.put_32(1) # The 32 bits integer
    return buf.data_array

Will get the packet as the OP wanted: [115, 116, 114, 105, 110, 103, 0, 1, 0, 0, 0]

Closing