GLTF: Return the error file path when an import fails. #94751
Conversation
fire
changed the title
fire
changed the title
| @@ -7416,7 +7416,7 @@ Error GLTFDocument::append_from_file(String p_path, Ref<GLTFState> p_state, uint | |||
|
|
|||
| Error err; | |||
| Ref<FileAccess> file = FileAccess::open(p_path, FileAccess::READ, &err); | |||
| ERR_FAIL_COND_V(err != OK, ERR_FILE_CANT_OPEN); | |||
| ERR_FAIL_COND_V_MSG(err != OK, err, vformat(R"(Can't open file at path "%s")", p_path)); | |||
There was a problem hiding this comment.
This is changing the return value from ERR_FILE_CANT_OPEN to err. Did you check that FileAccess::open wouldn't return other error codes, and that call sites for append_from_file don't make assumptions on the returned error?
There was a problem hiding this comment.
I think returning err is better, because it can provide more information. The file access functions usually return ERR_FILE_CANT_OPEN but can also return other values in other cases such as the function being called with invalid parameters.
As for the places that call append_from_file, the only internal usage just cares about err != OK, but this function is also exposed, so it technically changes behavior. However, I think it's a good change.
|
@RadiantUwU Can you rebase this pull request? |
|
Finally out of burnout, sorry for the long wait. |
|
Thanks! |
Allows the user to see which file failed to import and why, making it easier to spot import errors of
.blendfiles.