Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResourceLoader: Report appropriate error code when no suitable loader is found #99494

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

RandomShaper
Copy link
Member

New version of the reverted #97370.

One difference worth mentioning compared to the reverted PR is that, in order to make a simpler diff, reporting the file doesn't
exist is more prioritary than telling the extension is unknown. I don't think we have reasons to prefer one over the other.

E.g., loading thing.aaa, which doesn't exist:

  • Old PR: ERR_FILE_UNRECOGNIZED.
  • This PR: ERR_FILE_NOT_FOUND.

If the file exists, then this PR will then do report ERR_FILE_UNRECOGNIZED.

Fixes #95490.

@RandomShaper RandomShaper added bug topic:core cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Nov 21, 2024
@RandomShaper RandomShaper added this to the 4.4 milestone Nov 21, 2024
@RandomShaper RandomShaper requested a review from a team as a code owner November 21, 2024 12:52
@RandomShaper RandomShaper changed the title ResourceLoader: Report appropiate error code when no suitable loader is found ResourceLoader: Report appropriate error code when no suitable loader is found Nov 21, 2024
@@ -316,14 +316,34 @@ Ref<Resource> ResourceLoader::_load(const String &p_path, const String &p_origin
return res;
}

#ifdef TOOLS_ENABLED
if (Engine::get_singleton()->is_editor_hint()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restricting this to editor sessions (and not just editor builds) because at runtime many types are not registered with the importer and therefore the check below would be less reliable. That way, we have two behaviors only: editor and runtime, and not an in-between third one, that would make things unnecessarily complex.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give this a spin for beta2, so we have some time to get it tested in master first (beta1 is imminent so I wouldn't risk potentially introducing a regression).

@Repiteo Repiteo merged commit 0c0c45d into godotengine:master Jan 16, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 16, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:core
Projects
None yet
3 participants