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

Cannot instantiate File class to read CSV file #50831

Closed
Blackiris opened this issue Jul 25, 2021 · 7 comments · Fixed by #50841
Closed

Cannot instantiate File class to read CSV file #50831

Blackiris opened this issue Jul 25, 2021 · 7 comments · Fixed by #50841

Comments

@Blackiris
Copy link
Contributor

Blackiris commented Jul 25, 2021

Godot version

4.0.dev (fb821b8)

System information

Linux

Issue description

When trying to open a CSV file using this code:

extends Node

# Called when the node enters the scene tree for the first time.
func _ready():
	var file = File.new()
	file.open("res://file.csv", file.READ)

I get the following errors:

ERROR: Cannot get class 'File'.
   at: instantiate (core/object/class_db.cpp:529)
ERROR: Class type: 'File' is not instantiable.
   at: _new (modules/gdscript/gdscript.cpp:76)

The CSV file is set at Do not import.
The issue started from a commit 2 or 3 days ago, but it is hard to bisect it since many scene file formats had some changes which make them incompatible with older versions of Godot.

Steps to reproduce

Open the reproduction project and launch it

Minimal reproduction project

FileInstantiate.zip

@Blackiris Blackiris changed the title Cannot instantiate File Cannot instantiate File class to read CSV file Jul 25, 2021
@akien-mga akien-mga added this to the 4.0 milestone Jul 25, 2021
@qarmin
Copy link
Contributor

qarmin commented Jul 25, 2021

This is regression between b918c4c and fb821b8

I thinky that this commit could do this - #50511

@Blackiris
Copy link
Contributor Author

This is regression between b918c4c and fb821b8

I thinky that this commit could do this - #50511

Hey! Nice catch! Just tried to revert this commit and it works. It is now much easier to debug. Thanks!

@akien-mga
Copy link
Member

CC @aaronfranke

Haven't checked in depth yet but from the changes in #50511 to ClassDB, this part might have changed behavior:
Screenshot_20210725_120345

I think now we're always making a copy while before we were copying only when doing a validation.
But that doesn't necessarily relate to this bug.

@akien-mga
Copy link
Member

I think now we're always making a copy while before we were copying only when doing a validation.
But that doesn't necessarily relate to this bug.

Tested, that doesn't impact this bug indeed. I'll add a fix for this anyway in #50809.

@Blackiris
Copy link
Contributor Author

Blackiris commented Jul 25, 2021

Found it, it is here:

for (StringName &n : class_list) {
String s = String(n);
if (s.begins_with("_")) {
n = s.substr(1, s.length());
}
if (globals.has(n)) {
continue;
}
Ref<GDScriptNativeClass> nc = memnew(GDScriptNativeClass(n));
_add_global(n, nc);
}

More exactly this line:
Ref<GDScriptNativeClass> nc = memnew(GDScriptNativeClass(n));

You can see than the n value is erased here
n = s.substr(1, s.length());

Before it was,
Ref<GDScriptNativeClass> nc = memnew(GDScriptNativeClass(E->get()));
Which was the original value in the loop variable

Very small change, I will prepare a PR

@Blackiris
Copy link
Contributor Author

Blackiris commented Jul 25, 2021

Too bad, avoiding n to be erased is not enough... Got a new error

SCRIPT ERROR: Compile Error: Identifier not found: File
          at: GDScript::reload (res://new_script.gd:5)
ERROR: Method/function failed. Returning: ERR_COMPILATION_FAILED
   at: reload (modules/gdscript/gdscript.cpp:858)

Didn't try yet with PR #50809.
Don't know if @akien-mga, you could include the fix directly your PR. here is the diff:

------------------------ modules/gdscript/gdscript.cpp ------------------------
index 79cc90b92f..7015c1aa95 100644
@@ -1647,10 +1647,10 @@ void GDScriptLanguage::init() {
 	for (StringName &n : class_list) {
 		String s = String(n);
 		if (s.begins_with("_")) {
-			n = s.substr(1, s.length());
+			s = s.substr(1, s.length());
 		}
 
-		if (globals.has(n)) {
+		if (globals.has(s)) {
 			continue;
 		}
 		Ref<GDScriptNativeClass> nc = memnew(GDScriptNativeClass(n));

Worst case, I will have a look later today

@Blackiris
Copy link
Contributor Author

Ah found it, forgot to also change line:
_add_global(n, nc);

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.

3 participants