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

Script editor undo sometimes merges unrelated actions #77101

Closed
KoBeWi opened this issue May 15, 2023 · 10 comments · Fixed by #85325
Closed

Script editor undo sometimes merges unrelated actions #77101

KoBeWi opened this issue May 15, 2023 · 10 comments · Fixed by #85325

Comments

@KoBeWi
Copy link
Member

KoBeWi commented May 15, 2023

Godot version

4.1 5c653c2

System information

Windows 10 x64

Issue description

Example:

godot.windows.editor.dev.x86_64_mOiEC5hG9k.mp4

I started writing something, but made a mistake, so wanted to undo it. The undo registered this change and another change in a different line as the same action. So I either have to manually remove the wrong text (the one below) or write the correct text (the one above) again 😒

It happens quite frequently. I move the caret to another line and write something, but then it suddenly gets merged with what I wrote previously. Annoying stuff.

Steps to reproduce

  1. Write some code
  2. Write some more code
  3. Undo
    (I don't know what exactly triggers cases like above; at least it doesn't happen all the time)

Minimal reproduction project

N/A

@ajreckof
Copy link
Member

Normally it is only about time between two inputs. So maybe you are super fast at changing lines?
I'm not sure if it has been done or not but maybe we could enable customisation of the max time between two inputs.

Another idea would be to force stop merge on certain action, for example changing line. I'm not sure undoredo permits that for the moment so we would need to add the evolution to it first. But I think this would be the best solution (if this is really about Kobewi being way too fast at typing)

@KoBeWi
Copy link
Member Author

KoBeWi commented May 19, 2023

So maybe you are super fast at changing lines? [...] (if this is really about Kobewi being way too fast at typing)

Suffering from success 🥲

There's another case of this bug, which is 10000% more annoying:

godot_oX5CguFOJO.mp4

When I redo the text, the editor instead of staying at the change teleports me to the last caret position. It's extremely confusing and gives a false impression that you made some change in unrelated line. In the video above, my change is adding -=, but for whatever reason the scroll goes completely bonkers when I redo this change.

@gokiburikin
Copy link

gokiburikin commented Aug 12, 2023

I'm not sure undoredo permits that

Something about the way the code editor works already does this and the grouping time for the same action is way too lenient for me. I waste a ton of time fighting with this feature and would love to see customizable times. The delay between control + backspaces being grouped in the video below is egregiously long. At the very least changing line should break the grouping, not necessarily be its own action.

Godot_v4.1-stable_win64_2023-08-12_07-28-27.mp4

Edit to add more context on how often this happens to me with the style of coding I do. Here I'm just trying to change two lines to go back on a decision I made, but I make a typo I want to undo. Line 1 from orbital_velocity = orbit_position - position to orbital_velocity = (orbit_position - position) / delta and line 2 from movement_vector = orbital_velocity to movement_vector = orbital_velocity * delta.

Here's what it looks like in the script editor vs VSCode.
godot_undo.mp4
vscode_undo.mp4

I fat finger ( in place of * constantly so this happens all the time. I can of course just ctrl + backspace, but the behaviour of that in Godot is also annoyingly not what I'm used to (two ctrl + backspace deletes orbital_velocity * delta instead of * delta) so it's annoying either way.

@wareya
Copy link
Contributor

wareya commented Sep 18, 2023

Here's my take on this:

  1. Moving the cursor should always cause a split into a new undo operation.
  2. If the last action was split by moving the cursor, redoing that action should not put the cursor where the cursor movement put the cursor. Cursor movements should only be included in undo/redo actions if they're tied together with some later operation.
  3. Saving should always cause a split into a new undo operation.
  4. Switching between adding text and deleting text should always cause a split into a new undo operation.

@TheSofox
Copy link
Contributor

I managed to find the setting for how long it needs to stay idle before they make the next actions part of a new undo operation. It's in Project Settings->GUI -> Timers -> Text Edit Idle Detect (sec) (gui/timers/text_edit_idle_detect_sec). You'll need to enable "Advanced Settings" to see them. By default it's 3 seconds.

Aside from that, I agree with what's been said. Moving the cursor, either by mouse or keyboard, should make the next action part of a new undo operation.

@TheSofox
Copy link
Contributor

Created a pull request to solve this issue. Now if you add text on three different lines in quick succession, you'll need three undo commands to clear them.

The only exception is if you hold down backspace and delete multiple lines in one go. I was going to see about dividing that up into multiple operations, but then wasn't sure if that was necessary. In any event, every time you click to move the caret or move the caret via arrow keys, it'll split the undo operation.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 12, 2024

I see this issue again in 4.3 rc3

godot_aQSZTb9JbS.mp4

It's not as common though and might be related to selection.

EDIT:
Here's a better example:

jpQ81oH6Qg.mp4

Undo and redo are inconsistent.

@TheSofox
Copy link
Contributor

I need more detailed repo steps. I appreciate the video but it's hard to work out what's going on since I don't know what keys are being pressed. In the 2nd video I'm not certain which issue you're pointing to.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 19, 2024

I actually haven't encountered it recently. Maybe it's fixed again.
In any case, in the video I press Ctrl+Z when text is deleted and Ctrl+Y when text is restored. But yeah, the recordings aren't the best.

@TheSofox
Copy link
Contributor

Alright, well glad to hear it's low key at the very least. Let us know if it happens again.

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.

6 participants