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

inst_to_dict not support built-in script #69486

Closed
CsloudX opened this issue Dec 2, 2022 · 6 comments · Fixed by #69720
Closed

inst_to_dict not support built-in script #69486

CsloudX opened this issue Dec 2, 2022 · 6 comments · Fixed by #69720

Comments

@CsloudX
Copy link

CsloudX commented Dec 2, 2022

Godot version

v4.0.beta7.official [0bb1e89]

System information

Windows10, 64bit

Issue description

image

Steps to reproduce

/

Minimal reproduction project

InstToDictTest.zip

@MewPurPur
Copy link
Contributor

MewPurPur commented Dec 2, 2022

Might be a case of #6533?

@Chaosus Chaosus removed this from the 4.0 milestone Dec 2, 2022
@Chaosus
Copy link
Member

Chaosus commented Dec 2, 2022

This bug exists in the 3.x as well.

The check may be incorrect here and should be removed:

if (!p->path.is_resource_file()) {
r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
r_error.argument = 0;
r_error.expected = Variant::DICTIONARY;
*r_ret = Variant();
*r_ret = RTR("Not based on a resource file");
return;
}

@KoBeWi
Copy link
Member

KoBeWi commented Dec 2, 2022

This bug exists in the 3.x as well.

I tested in 3.5 and it works.

@Chaosus
Copy link
Member

Chaosus commented Dec 2, 2022

@KoBeWi Are you sure?:

image

You must test it with a built-in script, run the following project in 3.5 stable version of Godot:
Test.zip

@KoBeWi
Copy link
Member

KoBeWi commented Dec 2, 2022

Ok I think I misread the issue. It's indeed broken in both versions.

But I think this is just a limitation. The method requires a script path to work, otherwise you won't be able to de-serialize the object. It's not possible to obtain a built-in resource from external scene (it's just not implemented. You can't preload them for the same reason).

@Chaosus Chaosus added documentation and removed bug labels Dec 3, 2022
@Chaosus
Copy link
Member

Chaosus commented Dec 3, 2022

But I think this is just a limitation. The method requires a script path to work, otherwise you won't be able to de-serialize the object. It's not possible to obtain a built-in resource from external scene (it's just not implemented. You can't preload them for the same reason).

Then I suggest adding the note to the documentation of this method.

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.

5 participants