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);
	}