Expose Image's `is_valid()` method to scripting

[Bimbam360]

Describe the project you are working on

Just answering a forum post on how to hypothetically have realtime texture updating from third party apps. Hashed up this, which "works", but either I'm missing something obvious or there is no clean way to handle for 'CORRUPT' images (situationally I get could Error code 16, but in addition to the stderr output).

var filepath := "user://picture.png"
var filesize:=0

func _physics_process(_delta):
    if FileAccess.file_exists(filepath):
        var file = FileAccess.open(filepath, FileAccess.READ)
        if file and file.get_open_error() == OK:
            var modifiedsize = FileAccess.get_modified_time(filepath)
            if filesize != modifiedsize:
                print("File size changed, attempting to load image...")
                if file and file.get_open_error() == OK:
                    var buffer = file.get_buffer(file.get_length())
                    if buffer.size() > 0:
                        var image = Image.new()
                        var error = image.load_png_from_buffer(buffer) # Arbitrarily seems to fail with uncaught C errors. Feels like a bug tbh
                        if error == OK and image.is_empty() == false:
                            print("Image loaded and valid")
                            $TextureRect.texture = ImageTexture.create_from_image(image)
                            filesize = modifiedsize
                        else:
                            print("Error: Image is not valid or could not be loaded.")
                    file.close()
                else:
                    print("File is currently being written to or inaccessible.")

Describe the problem or limitation you are having in your project

It seems either impossible or not well documented how to capture these errors in GDScript:

given there is literally an image.is_valid() function in image.cpp, why do we not have this exposed in the Image class in gdscript?

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add Image.is_valid() function and/or ensure ERR_FILE_CORRUPT is correctly handled as an ERROR return value (or point out I missed something stupid).

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

N/A

If this enhancement will not be used often, can it be worked around with a few lines of script?

The workaround currently is to just ignore it. The code does work, just feels bootleg and surely validating an images as not-corrupt is useful for anyone streaming textures on the fly.

Is there a reason why this should be core and not an add-on in the asset library?

Improvement to existing core class.

[AThousandShips]

This method isn't in image.cpp, it's a method on Ref which is a holder for RefCounted, the equivalent in GDScript is if image: for image.is_valid() or if not image: for !image.is_valid() (though it should be image.is_null())

[Bimbam360]

Thing is the console error output seems to be caused by var error = image.load_png_from_buffer(buffer) which should just return an error code (all the nested ifs were to try and rule out everything else), and were I to check "if image:" it would be after this.

More I think about it maybe this was/is a design choice in that you can't confirm an image is valid until attempting to load it, and at an engine level failing to load an image due to corruption should always provide console error output?

[AThousandShips]

The error is still relevant and should be printed to make things clear, the return error is for further checking IMO

[AThousandShips]

But regardless we can't expose this method on Image or any RefCounted class as it would only be false if the object was invalid, so it would cause a call error

What we could do is expose a method on Variant but that'd be very similar to just using it in an if statement, i.e. booleanizing it, though it would vary a bit

But regardless the specific method here isn't a method of Image or RefCounted but of Ref, a container outside of GDScript, and doesn't have an equivalent in GDScript

[timothyqiu]

Add Image.is_valid() function and/or ensure ERR_FILE_CORRUPT is correctly handled as an ERROR return value (or point out I missed something stupid).

ERR_FILE_CORRUPT is an internal error returned by the PNG loader. You should check for ERR_PARSE_ERROR instead.

load_png_from_buffer() returns ERR_PARSE_ERROR on any error when parsing a PNG. This includes cases where the PNG loader thinks the file is corrupted.

[Bimbam360]

My point is more that regardless of how you use it, C++ errors abound. load_png_from_buffer() is returning an error code yes (the type of the error aside), but why then stderr output also. It makes it 'look' like it's not being correctly handled, because before you can even check if == OK it's spamming the console. This doesn't match other similar functions that return errors' behaviour to my knowledge, so why here.

[AThousandShips]

Plenty of other methods print errors and return an error message, for example:

• SceneTree.change_scene_to_file/change_scene_to_packed/reload_current_scene
• Node.rpc
• FileAccess.set_unix_permissions/set_hidden_attribute
• Object.emit_signal

Just for a very small part of methods that do

[timothyqiu]

It makes it 'look' like it's not being correctly handled

Then this is a misunderstanding of how the engine works :)

I too used to think that if a function returns an Error, then it is silent on failure. Apparently, there is no such convention in Godot.

It's indeed annoying sometimes since we currently don't have a standard way of silencing expected/handled errors. But in general, unlike exceptions, error codes are otherwise silent if not printed manually. So for many users this current behavior is helpful for debugging.

Also, the called function has no way of knowing whether the caller will handle the error code it returns, so all it can do is unconditionally print out the error message.