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

Code completion popup interrupts itself even if contents are the same #57027

Closed
wareya opened this issue Jan 21, 2022 · 7 comments · Fixed by #81633
Closed

Code completion popup interrupts itself even if contents are the same #57027

wareya opened this issue Jan 21, 2022 · 7 comments · Fixed by #81633

Comments

@wareya
Copy link
Contributor

wareya commented Jan 21, 2022

Godot version

3.4

System information

Windows 7

Issue description

Note: This bug is sensitive to the text editor's autocompletion settings. Please make sure that they're the default when testing.

When the idle autocomplete timer fires, it fully refreshes the autocomplete list, even if the contents are the same. This moves the cursor back up to the top of the list without a change in contents. The user might already be navigating the autocomplete list because there are many cases where it updates immediately upon input, so this is a usability problem.

IIRC this happens on master (4.x) as well.

Steps to reproduce

Type something that autocompletes, but where typing a little more will keep the same autocomplete list, e.g. Spatial.tra. Navigate the autocomplete list immediately. Wait for the code complete timer to fire. The cursor will move back up to the top. Add a character to the text, e.g. Spatial.tran, do the same thing, observe the same thing.

Video for clarity:

2022-01-20_22-37-15.mp4

Minimal reproduction project

No response

@KoBeWi
Copy link
Member

KoBeWi commented Apr 10, 2022

Can't reproduce in 4.0 (tested in alpha 6).

@KoBeWi KoBeWi closed this as completed Apr 10, 2022
@wareya
Copy link
Contributor Author

wareya commented Apr 10, 2022

Still happens in 4.0 (built from 83d2673). Also, I reported this against 3.4, so if it was fixed in 4.0 then the milestone should have been downgraded to 3.5 or 3.x, not closed.

godot_2022-04-10_19-06-33.mp4

This bug is sensitive to the text editor's completion settings. Please make sure that your settings (particularly involving parse delay) are default.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 10, 2022

Ok I didn't test enough before. I can reproduce this, but it isn't that easy to trigger (probably depends on how you use autocompletion though).

Also, I reported this against 3.4, so if it was fixed in 4.0 then the milestone should have been downgraded to 3.5 or 3.x, not closed.

Script editor and autocompletion have changed so much in 4.0 that 3.x would require a separate fix. If it's not known what PR fixed the issue then it would be very unlikely to happen.

@KoBeWi KoBeWi reopened this Apr 10, 2022
@KoBeWi KoBeWi moved this to To Assess in 4.x Priority Issues Apr 10, 2022
@KoBeWi KoBeWi moved this from To Assess to Todo in 4.x Priority Issues Apr 10, 2022
@wareya
Copy link
Contributor Author

wareya commented Apr 10, 2022

I can reproduce this, but it isn't that easy to trigger (probably depends on how you use autocompletion though).

I run into it several times per day, and it forces me to re-scroll through the autocomplete every time. How easy it is to trigger probably depends on your typing habits, but I would hesitate to describe it as "not that easy to trigger", because it happens to me so frequently.

Script editor and autocompletion have changed so much in 4.0 that 3.x would require a separate fix. If it's not known what PR fixed the issue then it would be very unlikely to happen.

IIRC, in 3.x it happens because of autocomplete timers piling up (at least with the immediate on-input autocomplete), and if 4.0 uses the same timer system (even if the rest of the system is different), then the fix would probably be similar even if it couldn't be rebased directly.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 21, 2023
@YuriSizov YuriSizov modified the milestones: 4.1, 4.x Jun 22, 2023
@matorin57
Copy link
Contributor

matorin57 commented Sep 14, 2023

I did some playing around and was able to repro. After putting some breakpoints it appears that CodeEdit::update_code_completion_options is getting called after CodeEdit::gui_input which then resets the selected text to 0.
It appears that the CodeEdit::update_code_completion_options is also called afterb CodeEdit::_filter_code_completion_candidates_impl was called and already filled in the options.
image

I was pretty easily able to reproduce but you have to interact with the down button pretty quickly. Its a race condition caused by initializing the list twice in a row. It seems consistent that CodeEdit::_filter_code_completion_candidates_impl is called then CodeEdit::update_code_completion_options called which in turn calls CodeEdit::_filter_code_completion_candidates_impl. It does appear that the input of the "character

I'm new to this project and am not fully aware of this entire flow so I don't have a fix in mind yet. Is anywhere aware of how this flow works and why?

Also I think @wareya was right in it being related to signals as the CodeEdit::update_code_completion_options is being called by a "code_completion_request" signal. Maybe there can be someway to check if the signal was caused by an "up/down" to then stop the cursor update?

@matorin57
Copy link
Contributor

Im guessing the signal can cause more options to be available from remote sources so prolly want to keep that as is. Maybe some logic could be added into CodeEdit::_filter_code_completion_candidates_impl to where it can detect if it made no changes, in which case it wouldn't reset the cursor to 0.

@matorin57
Copy link
Contributor

matorin57 commented Sep 14, 2023

Ok so I made a prototype for a change that would update the filtering logic to be more aware of the cursor https://github.com/matorin57/godot/tree/code-completion-popup

The basic idea is that instead of immediately deleting the old list, create a new list. Then after populating and sorting it, compare it and the old list up to the currently selected option. If none of the options up to the currently selected option have changed, then we don't reset the currently selected option to 0.

The code isn't fully cleaned up yet but figured would be good to get eyeballs on it while I'm cleaning it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants