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

JSON can not be loaded with ResourceLoader::load but the file can be correctly parsed #67630

Closed
PucklaJ opened this issue Oct 19, 2022 · 2 comments
Milestone

Comments

@PucklaJ
Copy link
Contributor

PucklaJ commented Oct 19, 2022

Godot version

4.0 beta3

System information

Garuda Linux

Issue description

There is a dedicated ResourceFormatLoader for JSON, but this loader can not load JSON files because the JSON class is not a Resource:

godot/core/io/json.cpp

Lines 582 to 609 in 61051a4

Ref<Resource> ResourceFormatLoaderJSON::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) {
if (r_error) {
*r_error = ERR_FILE_CANT_OPEN;
}
if (!FileAccess::exists(p_path)) {
*r_error = ERR_FILE_NOT_FOUND;
return Ref<Resource>();
}
Ref<JSON> json;
json.instantiate();
Error err = json->parse(FileAccess::get_file_as_string(p_path));
if (err != OK) {
if (r_error) {
*r_error = err;
}
ERR_PRINT("Error parsing JSON file at '" + p_path + "', on line " + itos(json->get_error_line()) + ": " + json->get_error_message());
return Ref<Resource>();
}
if (r_error) {
*r_error = OK;
}
return json;
}

This method can correctly load and parse a json file, but when it gets returned, the Ref<Resource> becomes null, because it can not be converted into a Resource. This is because of this:

godot/core/io/json.h

Lines 39 to 40 in 61051a4

class JSON : public RefCounted {
GDCLASS(JSON, RefCounted);

JSON is derived from RefCounted, but not from Resource. This seems to be somewhat indentional, but why is there a specific ResourceFormatLoader if it will never be able to load it?

I tried to change the base class of JSON to Resource and the ResourceFormatLoader can load it, but just changing the base class will probably not be enough and lead to other bugs. I just wanted to open this issue to point out that something needs to be done to fix this.

Related: #66820

Steps to reproduce

  1. Create a new project
  2. Create a JSON file
  3. There will probably already be a lot of errors, because the editor tries to load the file.
  4. Load it with ResourceLoader.load.
  5. See that the returned Resource is null, but the error is OK.

Minimal reproduction project

No response

pfertyk added a commit to pfertyk/godot that referenced this issue Oct 20, 2022
@pfertyk
Copy link
Contributor

pfertyk commented Oct 20, 2022

The problem (as noticed by @PucklaMotzer09 ) is that currently the inheritance tree looks like this:

Object
└── RefCounted
    ├── Resource
    └── JSON

This means that JSON cannot be loaded using ResourceLoader, because it's not actually a Resource.

One solution would be to make JSON a Resource, as initially suggested by @PucklaMotzer09 . Possibly we would have to move several types around, like FileAccess, which seems to be similar (also a file, but without a specific format).

Another solution is to make the ResourceLoader.load method return Ref<RefCounted>, which also seems like a huge change. Probably ResourceLoader would have to be renamed to RefCountedLoader :)

For now I'd like to submit a PR that makes JSON a Resource, and if we choose a different direction, please let me know and I'll amend it ;)

@akien-mga
Copy link
Member

Fixed by #71390.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants