Autocomplete sometimes places undesired quotes

[AeroBliss]

Tested versions

v4.2.1.stable.official [b09f793]

System information

Godot v4.2.1.stable - Linux Mint 21.2 (Victoria) - X11 - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 1080 (nvidia; 535.129.03) - AMD Ryzen 9 3900X 12-Core Processor (24 Threads)

Issue description

Autocomplete includes unnecessary quotes when using event from func _input(event) and load(). #81833 partially fixed autocomplete quotes reported in #48436 (as demonstrated in video below), but it still happens in this scenario with _input and load() (load() example not shown).

Steps to reproduce

Ignore line 9, was just experimenting.

2023-12-16.19-41-00.webm

Minimal reproduction project (MRP)

bugPOC.zip

[jsjtxietian]

Tested on Godot v4.3.dev (13a0d6e) - Windows 10.0.22621 - Vulkan (Mobile) - dedicated NVIDIA GeForce RTX 3060 Ti (NVIDIA; 31.0.15.4601) - 12th Gen Intel(R) Core(TM) i7-12700F (20 Threads)

Open the mrp and I did not reproduce this issue, seems like maybe there's other conditions for this bug to appear

[kaluluosi]

Tested versions

v4.2.1.stable.official [b09f793]

System information

Godot v4.2.1.stable - Linux Mint 21.2 (Victoria) - X11 - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 1080 (nvidia; 535.129.03) - AMD Ryzen 9 3900X 12-Core Processor (24 Threads)

Issue description

Autocomplete includes unnecessary quotes when using event from func _input(event). #81833 partially fixed autocomplete quotes reported in #48436 (as demonstrated in video below), but it still happens in this scenario with _input. It also happens with load().

Steps to reproduce

Ignore line 9, was just experimenting.

2023-12-16.19-41-00.webm

Minimal reproduction project (MRP)

bugPOC.zip

It seemes that because the InputEvent completion is hard code to return as constant with quotes.

[AThousandShips]

The added quotes are correct, it seems it doesn't handle the existing quotes correctly, or adds them twice, otherwise the result when not typing anything before requesting would be is_action_pressed(my_action)

[kaluluosi]

The added quotes are correct, it seems it doesn't handle the existing quotes correctly, or adds them twice, otherwise the result when not typing anything before requesting would be is_acrion_pressed(my_action)

I guess is because the CompletionItem Input.is_action_pressed return is totally different to InputEvent.is_action_pressed.

Input.is_action_pressed responds the completion item kind is 2 -> lsp::CompletionItemKind::Method

InputEvent.is_action_pressed responds the completion item kind is 21 -> lsp::CompletionItemKind::Constant

And I find something interesting in GodotScript LSP server.

While typing InputEvent.is_action_pressed. In the method _find_call_arguments：

And because the base is nil, so it is not considered as a method.

Here is where LSP server guess type. It don't work to InputEvent.

It seems that there has a hard code to force fix the issue of InputEvent not identified. It force return the input action options as CODE_COMPLETION_KIND_CONSTANT aka CompletionItemKind::Constant -> 21.

And this is the case of Input.is_action_presss：
The LSP server has identified the type of Input correctly. And return the CODE_COMPLETION_KING_FUNCTION aka ComplectionItemKind::Method ->2.

Questions

1. Why InputEvent so especial then it's type cannot be identified by _guess_expression_type ?
2. Which is right kind of CompletionItem to return ? It seems that as a parameter and action string, it should return as Constant kind. But Input.is_action_pressed returns Method kind.
3. Why hard code the options of CompletionItem of InputEvent.*action*. Is that means i can fix it in the hard code part too? That sounds not good.

[AThousandShips]

Can't replicate this without the LSP so can't test it properly unfortunately

[kaluluosi]

Can't replicate this without the LSP so can't test it properly unfortunately

You need two vscode projects godot and godot-tools to test it.

I have test it out. and find three confuse questions above.

If I simple make InputEvent return COMPLETION_KIN_FUNCTION. It works just like Input, no double quotes both Godot Editor and VSCode.

I don't know how _guess_expression_type works.
I guest it is use the ClassDB or something class meta data. Somewhere define's the method Input.is_action_pressed ‘s signature. But InputEvent.is_action_pressed lack its.

[AThousandShips]

It's probably not the complete solution, but I have some ideas, if I can set up LSP I can test them out when I find the time

I think instead this line should be changed:

godot/modules/gdscript/language_server/gdscript_text_document.cpp

Line 346 in 0bcc0e9

item.insertText = item.label.quote(quote_style);

To:

item.insertText = item.label.unquote().quote(quote_style);

As is done elsewhere

[AeroBliss]

Just want to make it known that this bug also happens with load().

[kaluluosi]

Just want to make it known that this bug also happens with load().

I have make my try to fix it.
You can donwload Godot Editor from artifact of my action build to have a test.

https://github.com/kaluluosi/godot/actions/runs/7628380270

windows edtior download

I referred to @AThousandShips's suggestion to judge and clean up the quotes for specific kinds in the resolve function of gdscript_text_document.cpp.
To avoid unexpected issues with other auto-completion prompts, I only made fixes within lsp::CompletionItemKind::Constant and lsp::CompletionItemKind::File. It's a pretty conservative fix. Although I think the fundamental problem lies elsewhere. It'll have to do for now.

I also noticed that the triggerCharacters for LSP didn't include "(", which caused VSCode's "(" to not trigger completion, so I added it.

This is the test:

test-2.mp4

test-1.mp4

Finally

I think the LSP Server shouldn't include editor Preferences behavior in its response results, it should be more pure. Whether to insert parentheses after the method and whether to automatically close the parentheses should be editor behavior. But because GodotScript's LSP gets the data processed by Godot Script Editor, it inevitably brings in the inserted values processed by the editor. So now in gdscript_text_completion.cpp, we have to reverse the processing done by Godot Script Editor.

And the GDScript LSP is so complex. 😂

Based on my research and observation, I understand why the is_action* method of the event variable in _input(event) cannot pop up the action autocomplete, but Input can.

The key function for a method of an instance variable within a script to pop up its parameter autocomplete is this:

godot/core/object/object.h

Line 950 in 74c32fa

virtual void get_argument_options(const StringName &p_function, int p_idx, List<String> *r_options) const;

However, this function will only be called by the Godot Script Editor if the following conditions are met:

1. The object instance of the current script (class) must be in the editor's memory, in other words, the scene to which the script is attached must be in the open editing state.
2. Alternatively, the script is a global singleton, Autoload.
3. When the player accesses the script of the scene being edited, the autocomplete context.base is equal to the current node's instance in the editor. Then, naturally, it can run to call get_argument_options.
   

   godot/modules/gdscript/gdscript_editor.cpp

   Lines 2614 to 2639 in 0bcc0e9

   	if (ClassDB::get_method_info(class_name, p_method, &info)) {
   	method_args = info.arguments.size();
   	if (base.get_type() == Variant::OBJECT) {
   	Object *obj = base.operator Object *();
   	if (obj) {
   	List<String> options;
   	obj->get_argument_options(p_method, p_argidx, &options);
   	for (String &opt : options) {
   	if (opt.is_quoted()) {
   	opt = opt.unquote().quote(quote_style); // Handle user preference.
   	}
   	ScriptLanguage::CodeCompletionOption option(opt, ScriptLanguage::CODE_COMPLETION_KIND_FUNCTION);
   	r_result.insert(option.display, option);
   	}
   	}
   	}
   	if (p_argidx < method_args) {
   	PropertyInfo arg_info = info.arguments[p_argidx];
   	if (arg_info.usage & (PROPERTY_USAGE_CLASS_IS_ENUM | PROPERTY_USAGE_CLASS_IS_BITFIELD)) {
   	_find_enumeration_candidates(p_context, arg_info.class_name, r_result);
   	}
   	}
   	r_arghint = _make_arguments_hint(info, p_argidx);
   	}

Since the method's parameter variable is static during editing, it will not execute inside, so the parameter variable event in the method body can only infer the type and cannot obtain the instance. Therefore, it can ultimately obtain the autocomplete of the method through ClassDB, but it cannot obtain the autocomplete of the method's parameters. Because without an instance object, it cannot call get_argument_options.

However, in order to obtain the development experience, we have to hard code the addition of the action autocomplete for InputEvent.

godot/modules/gdscript/gdscript_editor.cpp

Lines 2716 to 2730 in 0bcc0e9

	if (p_argidx == 0 && method_args > 0 && ClassDB::is_parent_class(class_name, SNAME("InputEvent")) && p_method.operator String().contains("action")) {
	// Get input actions
	List<PropertyInfo> props;
	ProjectSettings::get_singleton()->get_property_list(&props);
	for (const PropertyInfo &E : props) {
	String s = E.name;
	if (!s.begins_with("input/")) {
	continue;
	}
	String name = s.get_slice("/", 1);
	ScriptLanguage::CodeCompletionOption option(name, ScriptLanguage::CODE_COMPLETION_KIND_CONSTANT);
	option.insert_text = option.display.quote(quote_style);
	r_result.insert(option.display, option);
	}
	}

The purpose of get_argument_options is to make the Godot Script Editor more closely integrated with the scene editor. In order for the script editor to access some data of the project and scene during scene editing to generate autocomplete options, it had to be designed as a class virtual method, because class static methods cannot access the scene tree.

That's about it.

PS: I have looked at many kinds of get_argument_options and found that none of them actually access the scene tree. I think it should be fine to design them as class static virtual methods.

I'm wrong, sorry. It actually need to visit scene tree. 😢

godot/scene/main/node.cpp

Lines 3102 to 3113 in 74c32fa

	void Node::get_argument_options(const StringName &p_function, int p_idx, List<String> *r_options) const {
	String pf = p_function;
	if (p_idx == 0 && (pf == "has_node" || pf == "get_node" || pf == "get_node_or_null")) {
	_add_nodes_to_options(this, this, r_options);
	} else if (p_idx == 0 && (pf == "add_to_group" || pf == "remove_from_group" || pf == "is_in_group")) {
	HashMap<StringName, String> global_groups = ProjectSettings::get_singleton()->get_global_groups_list();
	for (const KeyValue<StringName, String> &E : global_groups) {
	r_options->push_back(E.key.operator String().quote());
	}
	}
	Object::get_argument_options(p_function, p_idx, r_options);
	}

[Meorge]

As discussed in my issue submitted to the VS Code plugin, I've consistently experienced this issue with the dollar-sign get_node() shortcut, as seen here:

I wouldn't be able to test the current PR until later today at the soonest, but it would be great if it could cover this scenario as well.

[kaluluosi]

As discussed in my issue submitted to the VS Code plugin, I've consistently experienced this issue with the dollar-sign get_node() shortcut, as seen here:

I wouldn't be able to test the current PR until later today at the soonest, but it would be great if it could cover this scenario as well.

In my PR version it is fine.

Clipchamp.mp4

You can download my PR version Godot Editor artifact. Here is the link to access the artifact: link goes here.
Scroll down to the bottom to check out the build artifacts, find the Godot Editor download, and give it a try.

[Meorge]

Perhaps I'm just not seeing the download link correctly, but it looks like the artifacts are expired and can't be downloaded anymore:

Also, in your video, I'm noting that none of the names you have it autocomplete have spaces in them. From what I've seen this seems to be a problem when there are spaces in the name, and as a result the language server inserts quotes around its suggestion. Here's a video demonstrating this:

godot.language.server.bug.-.double.quotes.mp4

[kaluluosi]

Hmm... it looks like the artifact build has expired, so you'll have to check out my branch and build it yourself.

[Meorge]

I just tested the branch you linked, and it looks like it still has the same problem as shown in the video above.

godot.language.server.bug.-.double.quotes.on.fix.branch.mp4

[kaluluosi]

I just tested the branch you linked, and it looks like it still has the same problem as shown in the video above.

godot.language.server.bug.-.double.quotes.on.fix.branch.mp4

The suggestion of quote seems not provided by Godot LSP. Godot LSP not remember what you typed before. The quote one look like the VSCode's record.

[AeroBliss]

I just tested the branch you linked, and it looks like it still has the same problem as shown in the video above.
godot.language.server.bug.-.double.quotes.on.fix.branch.mp4

The suggestion of quote seems not provided by Godot LSP. Godot LSP not remember what you typed before. The quote one look like the VSCode's record.

Should a new issue be opened on the Godot VS Code extension repository then?

[Meorge]

Should a new issue be opened on the Godot VS Code extension repository then?

I've got one open here: godotengine/godot-vscode-plugin#644

[geekley]

Not sure if this is the right issue to ask this, but... I wish there was a setting for Godot's LSP to never add any extra parentheses, quotes or any other symbol when accepting auto-completion suggestion of a word.

If I selected a word, I want to add just the word. Reasons from godotengine/godot-vscode-plugin#152 (comment):

Yes, this has been bothering me for quite a while.

Other languages in VSCode don't add any sort of parentheses on auto-completion, and this is what I expected for GDScript too. It's one of those things where doing too much automatically will be more trouble than help, and there's many reasons why:

• I don't always want a ( after accepting function suggestion. Often I want to pass its callback, so you shouldn't assume I want the parentheses and make me have to delete it, even if calling happens more often.
• Since it adds just a (, it makes the parentheses unbalanced.
• It's more trouble to delete the extra unbalanced (. For example, accepting a suggestion for function bar in this code: pass_callback(bar(|) (where | is the cursor), pressing backspace I want to delete the extra ( only, but it deletes both (). This is why is better to not assume and "try to help" too much adding parentheses.
• When I accept an auto-complete suggestion for a function, on other languages I type ( manually and automatically get the function signature popup without having to do Ctrl+Shift+Space. On GDScript, this doesn't happen, because it auto-fills the ( without triggering the parameter hints. So I still have to manually press MORE keys anyway (and add the closing ) afterwards).
• Being inconsistent with the behavior in other languages in VSCode is also a reason by itself.

So, please add at least an option to just not add any parentheses automatically.

[atirut-w]

Sounds like a different issue to me.