Mention that ERR_FILE_EOF is not always an error

[andreymal]

Let's look at a naive implementation of error handling:

var fp := FileAccess.open("res://myfile.txt", FileAccess.READ)
var data := fp.get_buffer(65536)
if fp.get_error():
    print("PANIK I/O IS BROKEN YOUR HDD IS DYING AAAAAAA")

If the file is small enough, get_buffer reaches EOF, and get_error returns 18 (ERR_FILE_EOF).

But unless the user expects exactly 65536 bytes in the buffer, it's not an error, and ERR_FILE_EOF can be ignored.

This may be obvious for experienced users, but it's easy to overlook it.

I suggest mentioning somewhere (probably in the FileAccess documentation) that reaching EOF when reading dynamically sized data (i.e. when using get_buffer, get_line and probably other methods) doesn't always indicate something bad and can be safely ignored in some cases.

[AThousandShips]

This isn't true, it is an error to try read more than is available in the file, you should use some other method to read from the file, this error tells you that you are, in fact, reading too much, that's what the error is for, reading like this is not proper IMO and shouldn't be encouraged

[andreymal]

@AThousandShips why? It is never interpreted as an error in the read function in Linux and Windows, for example. In particular, read(2) states explicitly:

It is not an error if this number is smaller than the number of bytes requested

Is there a good reason why Godot shouldn't follow the same logic?

[AThousandShips]

Well those are not the same functions

As I said I have an issue with suggesting behavior that I don't think is good code, and the method documentation says:

Returns next length bytes of the file as a PackedByteArray.

Reading like this makes it riskier and makes error handling harder, instead people should, in my opinion, read in a way that considers the data remaining

Also it's just us here, please don't ping me 🙂

[andreymal]

This implies calling get_length() (its implementation in FileAccessUnix is quite slow, by the way) and calculating the buffer size on the fly, which probably makes the code even more error-prone and difficult to maintain 🤔

And get_line() can't know the data remaining

[AThousandShips]

And get_line() can't know the data remaining

Yes, but that's different, it isn't provided an explicit length, you can do this way with providing a longer length, but I'd say that saying "don't worry" sends the wrong message, but again, taht's my opinion on this. We return an error, that error should be respected

And especially, IMO, if you are reading in this way you are expected to know what you do

[andreymal]

Hm, there is actually a warning in FileAccessMemory::get_buffer:

	if (read < p_length) {
		WARN_PRINT("Reading less data than requested");
	}

But other implementations of FileAccess::get_buffer don't have this warning, which is inconsistent.

Seems like a clear choice needs to be made:

• If EOF is an error, then this warning should probably be added to other implementations
• If EOF is not always an error and can be ignored in some cases, then this warning should probably be removed to make FileAccessMemory consistent with other implementations and not annoy users with potentially useless warnings