`StreamPeerBuffer` fails to return only a part of the string previously stored on it

[pafuent]

Tested versions

• Reproducible in: 4.3.stable

System information

Godot v4.3.stable - macOS 14.6.1 - GLES3 (Compatibility) - Intel(R) Iris(TM) Plus Graphics OpenGL Engine - Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz (8 Threads)

Issue description

If you put a string or an utf8 string in a StreamPeerBuffer and then you try to get just a part of it you will get an empty or corrupted string.

Steps to reproduce

Here is a small script to reproduce the issue.

extends Node

# Called when the node enters the scene tree for the first time.
func _ready() -> void:
    var buff = StreamPeerBuffer.new()
    buff.put_string("Hello, World!!!")
    buff.seek(0)
    print("->" + buff.get_string(5) + "<-")

    buff.clear()
    buff.put_utf8_string("Hello✩, World✩!!!")
    buff.seek(0)
    print("->" + buff.get_utf8_string(8) + "<-")

This script will print:

-><-
-><-

Minimal reproduction project (MRP)

streampeerbuffergetpartialstringbug.zip

[timothyqiu]

This is not a bug I believe.

As already documented, get_string() and get_utf8_string() are the reverse of put_string() and put_utf8_string() only when size is not specified.

This allows parsing strings from a raw buffer directly.

In your use case, the entire string should be read first, then call left(), right(), or substr() to get a part of it.

[pafuent]

Maybe I'm missing something, but if the intended purpose of those functions are to be only used when you are providing the exact bytes of the string that you want to read or -1, them seems that bytes parameter is not adding value. Besides of that, my understanding that this could be a bug comes from the actual implementation of those methods: they are ignoring the stored string length as it its and are using those bytes as part of the string that are reading, so what you get is the string length encoded as the first bytes of the string that you are really trying to read.

[timothyqiu]

To be clear:

• peer.get_string() is the reverse of peer.put_string(str)
• peer.get_string(length) is the shortcut of PackedByteArray(peer.get_data(length)).get_string_from_ascii()

[pafuent]

Sorry to insist with this, but for me this is a bug. The issue here is peer.get_string(length) is behaving on two different ways if I call it with a length parameter or not. If I call it without the parameter, it will read 32 bits from the peer before reading the string, but if I call it with a parameter it will directly read that amount of bytes from the peer. In order to know that fact, is needed to go and read implementation of the method. For me, get_string, with or whiteout a parameter should behave in a predictable way, as being the reverse of put_string in both cases

[timothyqiu]

In order to know that fact, is needed to go and read implementation of the method.

All you need to do is to read the manual:

https://github.com/godotengine/godot/blob/e2dd56bea7a1aa13fe74fd6eac049e4cbde0434c/doc/classes/StreamPeer.xml#L69-L75

Sorry to insist with this, but for me this is a bug.

This is bad API design, as it's easy to misuse.

However, bug is something that's not behaving as described in the documentation. ^{this is just my opinion of course}

[pafuent]

Now I see it, maybe my definition of bug is too broad.

This is bad API design, as it's easy to misuse.

I agree that this is a bad API design. What should I do to help? Should I close this bug and create another kind of issue? Should I keep this open until more people see it and we reach to a consensus about what we should do?

[timothyqiu]

I think you can leave it open.

To be honest, I don't think there would be any chance of changing the behavior of get_string() as it breaks compatibility, at least for 4.x.

In case you can find a name that everyone is happy with, though I think it's very hard as this is very basic functionality, it might be possible to mark the current get_string() as deprecated, in favor of two separate new functions.