godot
godot copied to clipboard
Fix `Find in Files` Search Results cannot open builtin script
Fix #74145
I don't think this is best way to do this but this is all I can come up with.
Suggestions for better way are welcome.
Yeah this is not a correct way to do this.
- You are instantiating the scene, which is rather overkill. And unreliable, because the script can be inside e.g. exported property or attached to a resource
- You are not freeing the nodes afterwards (this is easy to fix tho)
- It doesn't seem to work correctly with multiple scripts in scene. I clicked a result, but it opened me wrong script
- You still need to open the scene when you open the script. Built-in resources can't be edited outside their scene, because any changes you do won't be saved
The solution isn't really bad, but perfectly it should work without creating the nodes. I don't know if there is any way to load the resources from PackedScene without instantiating it.
However, I once opened a proposal for the same problem: https://github.com/godotengine/godot-proposals/issues/6032 My idea was that built-in GDScripts could be mapped to lines in the file and cached, but tbh I'm not sure where to put that cache. Maybe loading the scene each time is fine, but instead of loading as resource, open it as text instead and manually find the line?
How it could potentially work:
- User searches for "something"
- Editor finds it in line 79 of a tscn
- User clicks that line
- Editor opens the tscn as a text file
- It travels the file line-by-line and looks for GDScript sub-resource headers
- If it finds one, it parallelly counts the current line in the script
a. If it reaches line 79, it opens the scene + script on the counted script line
b. If it reaches end of the script (line ends with unescaped
"
), it looks for the next script until line 79
@KoBeWi I followed your suggestion and rewrite the logic. Can you check it again?
You still need to open the scene when you open the script. Built-in resources can't be edited outside their scene, because any changes you do won't be saved
I could not open both the scene and the script.
EditorNode::get_singleton()->load_scene(fpath);
and ste->goto_line_selection
don't want to work together.
First time clicking will open the scene. And Second Click will open the script file
EditorNode::get_singleton()->load_scene(fpath); and ste->goto_line_selection don't want to work together.
Pretty sure this can be solved with call_deferred()
.
tested ste->call_deferred(SNAME("goto_line_selection"), line_number, begin, end);
and it doesn't seem to work.
Scene opened. Builtin Script opened. But Line Selection doesn't work. Caret is at the top.
So I changed back to normal and tested EditorNode::get_singleton()->call_deferred(SNAME("load_scene"), fpath)
it doesn't seem to work either.
idk I tested and it works, but the editor does not center on the selected line. EDIT: External scripts are not centered either, so probably fine like this for now.
I checked the code and it looks fine. I found one minor problem, other suggestions are just comment style fixes.
Thanks for suggestions. Updated.
Can't we use ResourceFormatLoaderText
to parse the tscn and extract the GDScript subresource instead of reimplementing an ad hoc parser?
Edit: Note that I'm not sure this loader specifically is suitable, it just looks to me like we're reimplementing stuff which Godot can already do. But I see previous discussion seemed to propose doing this intentionally.
I will look into it now.
Using ResourceFormatLoaderText involves parsing the whole file. The current approach is much faster.
I am experimenting VariantParser::parse_tag_assign_eof
approach
while(true){
String assign;
Variant value;
Error error = VariantParser::parse_tag_assign_eof(&stream, lines, error_text, next_tag, assign, value, nullptr);
if(error == Error::ERR_FILE_EOF){
break;
}
if (next_tag.name != "sub_resource") {
continue;
}
if(!next_tag.fields.has("type")){
continue;
}
print_line("TYPE");
print_line(next_tag.fields["type"]);
if(next_tag.fields["type"] != "GDScript"){
continue;
}
if(!next_tag.fields.has("id")){
continue;
}
scr_id = next_tag.fields["id"];
It can work though.
But if
statements are the same as using [sub_resource type=\"GDScript\" id=\"
Let's go with the current approach then :)
Thanks!
Cherry-picked for 4.0.2.