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

Review TextFile name, exposure, implementation and usage for 4.0 #36297

Closed
Anutrix opened this issue Feb 17, 2020 · 4 comments · Fixed by #51845
Closed

Review TextFile name, exposure, implementation and usage for 4.0 #36297

Anutrix opened this issue Feb 17, 2020 · 4 comments · Fixed by #51845

Comments

@Anutrix
Copy link
Contributor

Anutrix commented Feb 17, 2020

Bugsquad edit: Issue repurposed to track necessary changes to TextFile as per #36297 (comment)


Godot version:
Godot Engine v4.0.dev.custom_build 705ad94

OS/device including version:
64 bit Windows 10 Enterprise 1909.

Issue description:
When opening any project, the console is spammed with

ERROR: Cannot get class 'TextFile'
   at: (core\class_db.cpp:356)

quite a few times along with some other errors(I think they are mentioned in other issues already).

Steps to reproduce:
Just open any project.
Simple project and console log attached.

Minimal reproduction project:
test.zip
test.log.txt

@akien-mga
Copy link
Member

akien-mga commented Feb 17, 2020

Regression from #36182.

This is likely because of #31927 which expects TextFile to be a usable resource type. See also #21787 and #24187.

Discussion from IRC:

13:19 <Akien> reduz: We have a TextFile resource type used by the script editor to load plain text files, and it's currently exposed in the documentation, which confuses users as it's useless for scripting.
13:19 <Akien> reduz: kobewi tried to solve that by removing the `GDCLASS(TextFile, Resource)`, which works, but now ClassDB whines about TextFile being missing.
13:19 <Akien> Is there a proper way to hide such an editor-internal resource from the public API?
13:20 <reduz> just rename it to EditorTextFile
13:20 <reduz> like everything editor
13:20 <reduz> so it wont be confusing any longer
13:22 <Akien> That makes sense yeah. But most Editor* classes are still useful for users to make editor plugins, while EditorTextFile would just be an empty "Internal resource, don't use" class.
13:24 <reduz> actually
13:24 <reduz> if you don't register it to use it
13:24 <reduz> it won't appear in the documentation
13:24 <reduz> of course if you want it for a resource loader, there is nothing you can do
13:24 <reduz> but i dont know how text files are opened. If they are as resources, you can' t hide it or not register it
13:24 <Akien> So I keep GDCLASS, but I remove the register_class?
13:25 <reduz> if you remove register_class it will move as long as it is not used as a resource to load or save files
13:25 <reduz> er i mean it will work
13:25 <reduz> but also do rename it to EditorTextFile
13:27 <reduz> i am finding so many broken usages of poolvector by contributors
13:27 <reduz> which work just by chance
13:27 <Akien> Hm, ResourceImporterCSV uses TextFile, so I can't unregister it.
13:28 <reduz> that sounds like hack
13:28 <Akien> Yeah
13:28 <reduz> should not be loaded as a resource, although i have no idea how to do it otherwise
13:28 <reduz> will have to talk to Paulb23 about it
13:28 <Akien> Yeah it's a hack added in https://github.com/godotengine/godot/commit/7ac0239afa5
13:29 <reduz> it should be an editor only class

We could simply revert #36182 to fix the issue, but I think we need to address the combined hacks that we have here around TextFile, and both the UX and API issues that it creates (users try to use it since it's in the public scripting API, but it's not usable. You can create a TextFile resource, which lets you edit it as text, but it will fail if it's not a valid .tres, etc.)

@Anutrix
Copy link
Contributor Author

Anutrix commented Feb 17, 2020

One part of solution is to rename it to InternalTextFile instead of EditorTextFile thus allowing Internal* category of similar classes to be created in future if it's needed.

@partyvaper
Copy link

partyvaper commented Feb 17, 2020

When opening project, this is the last I see in console, then it all crashes.

ERROR: Cannot get class 'TextFile'.
   at: (core\class_db.cpp:356)
format: 1 size 128, 144 mipmaps: 0
CrashHandlerException: Program crashed
PS D:\Git\godot\bin> Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] <couldn't map PC to fn name>
[1] <couldn't map PC to fn name>
[2] GDScriptParser::_type_from_variant (D:\git\godot\modules\gdscript\gdscript_parser.cpp:5864)
[3] GDScriptParser::_parse_expression (D:\git\godot\modules\gdscript\gdscript_parser.cpp:532)
[4] GDScriptParser::_parse_and_reduce_expression (D:\git\godot\modules\gdscript\gdscript_parser.cpp:2017)
[5] GDScriptParser::_parse_class (D:\git\godot\modules\gdscript\gdscript_parser.cpp:5043)
[6] GDScriptParser::_parse (D:\git\godot\modules\gdscript\gdscript_parser.cpp:8479)
[7] GDScriptParser::parse (D:\git\godot\modules\gdscript\gdscript_parser.cpp:8582)
[8] GDScript::reload (D:\git\godot\modules\gdscript\gdscript.cpp:574)
[9] ResourceFormatLoaderGDScript::load (D:\git\godot\modules\gdscript\gdscript.cpp:2357)
[10] ResourceLoader::_load (D:\git\godot\core\io\resource_loader.cpp:270)
[11] ResourceLoader::load (D:\git\godot\core\io\resource_loader.cpp:401)
[12] ScriptEditor::set_window_layout (D:\git\godot\editor\plugins\script_editor_plugin.cpp:2711)
[13] ScriptEditorPlugin::set_window_layout (D:\git\godot\editor\plugins\script_editor_plugin.cpp:3597)
[14] EditorData::set_plugin_window_layout (D:\git\godot\editor\editor_data.cpp:858)
[15] EditorNode::_load_docks (D:\git\godot\editor\editor_node.cpp:4297)
[16] EditorNode::_sources_changed (D:\git\godot\editor\editor_node.cpp:703)
[17] MethodBind1<EditorNode,bool>::call (D:\git\godot\core\method_bind.gen.inc:867)
[18] Object::call (D:\git\godot\core\object.cpp:921)
[19] Object::emit_signal (D:\git\godot\core\object.cpp:1217)
[20] Object::emit_signal (D:\git\godot\core\object.cpp:1275)
[21] EditorFileSystem::_notification (D:\git\godot\editor\editor_file_system.cpp:1175)
[22] EditorFileSystem::_notificationv (D:\git\godot\editor\editor_file_system.h:108)
[23] Object::notification (D:\git\godot\core\object.cpp:933)
[24] SceneTree::_notify_group_pause (D:\git\godot\scene\main\scene_tree.cpp:976)
[25] SceneTree::idle (D:\git\godot\scene\main\scene_tree.cpp:526)
[26] Main::iteration (D:\git\godot\main\main.cpp:2066)
[27] OS_Windows::run (D:\git\godot\platform\windows\os_windows.cpp:3177)
[28] widechar_main (D:\git\godot\platform\windows\godot_windows.cpp:162)
[29] _main (D:\git\godot\platform\windows\godot_windows.cpp:186)
[30] main (D:\git\godot\platform\windows\godot_windows.cpp:196)
[31] __scrt_common_main_seh (d:\agent\_work\3\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[32] BaseThreadInitThunk
-- END OF BACKTRACE --

@akien-mga
Copy link
Member

Reverting #36182 with #36317. I'll keep this issue open and repurpose it to handle necessary changes to TextFile and the way we handle "loading" CSV using it: #36297 (comment)

@akien-mga akien-mga changed the title ERROR: Cannot get class 'TextFile' in console. Review TextFile name, exposure, implementation and usage for 4.0 Feb 18, 2020
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