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

Godot 4: a click on an animation name can paste name of animation previously renamed on SpriteFrames #55769

Closed
gustavi opened this issue Dec 9, 2021 · 9 comments · Fixed by #68728

Comments

@gustavi
Copy link

gustavi commented Dec 9, 2021

Godot version

v4.0.dev.calinou [4f29823]

System information

Windows 10 64bit - Vulkan API 1.2.141 - GeForce GTX 1660 SUPER

Issue description

A video is better than words:

2021-12-09.21-58-46.mp4

Note: I only use mouse to perform this bug (I'm not pasting anything with my keyboard).

Godot 3.4 and 3.4.1-rc2 are not affected by this bug. All official builds are affected.

Steps to reproduce

  1. Go to SpriteFrames menu
  2. Create an animation named "a"
  3. Create an animation named "b"
  4. Double click on "a" for editing the name
  5. Click on "b"
  6. "b" should be renamed "a 1"

Minimal reproduction project

bug-click-animation-name.zip

@jmb462
Copy link
Contributor

jmb462 commented Feb 13, 2022

The bug concerns Tree node itself (not only SpriteFrames)
bug

While renaming, when clicking on another item popup_edited_item is updated before the rename is validated.

@gustavi gustavi changed the title SpriteFrames : a click on an animation name can paste name of animation previously renamed Godot 4: a click on an animation name can paste name of animation previously renamed on SpriteFrames Mar 15, 2022
@KoBeWi
Copy link
Member

KoBeWi commented May 2, 2022

Looks like #42419, except the "tree" in SpriteFrames editor is an ItemList.

@KoBeWi KoBeWi moved this to To Assess in 4.x Priority Issues May 2, 2022
@KoBeWi KoBeWi moved this from To Assess to Todo in 4.x Priority Issues May 2, 2022
@Rindbee
Copy link
Contributor

Rindbee commented Jun 12, 2022

I think I already know about the cause of this problem.
The reason is similar to #60842 (comment)

I added a print code to TreeItem::set_text and the log is as follows:
captrue

void SpriteFramesEditor::_update_library(bool p_skip_selector) {

In this method, the entire tree is rebuilt. And this method may be called for a small change.

@anvilfolk
Copy link
Contributor

anvilfolk commented Nov 15, 2022

Cannot reproduce on master. Seems like it was fixed by whatever fixed #42419?

2022-11-15.15-22-17.mp4

@akien-mga
Copy link
Member

Thanks for testing! Closing.

Repository owner moved this from Todo to Done in 4.x Priority Issues Nov 15, 2022
@Rindbee
Copy link
Contributor

Rindbee commented Nov 15, 2022

Sorry, this issue doesn't seem to be resolved yet in cfb9cae.

0.mp4

@KoBeWi KoBeWi reopened this Nov 15, 2022
@anvilfolk
Copy link
Contributor

I'm recompiling right now to test again, but it looks like you may have double-click selected the name and dragged it onto the other animation name?

@anvilfolk
Copy link
Contributor

anvilfolk commented Nov 15, 2022

Ok, here are some more details.

If you select a node, enter rename mode, then click some other node after a bit of time has passed, the bug does not occur.

If you double-click a node and immediately after click another node, the second node will become "selected". You can see it because it becomes highlighted. In that situation, clicking again anywhere will rename the last node selected.

Untitled

It will look like this: original renamed node is selected and in focus. Second clicked element is selected and not in focus.

Also, @marzecdawid was working on some Tree stuff, so thought I'd give him a tag :)

@Rindbee
Copy link
Contributor

Rindbee commented Nov 15, 2022

It seems that the results returned by Tree::get_edited() and Tree::get_selected() are inconsistent.

Edit:

TreeItem *selected = animations->get_selected();
ERR_FAIL_COND(!selected);
edited_anim = selected->get_text(0);

TreeItem *edited = animations->get_edited();
if (!edited) {
return;
}
String new_name = edited->get_text(0);
if (new_name == String(edited_anim)) {
return;
}

The comparison here is the text in edited and selected, so it should be required that edited and selected are the same.

@TokageItLab TokageItLab moved this from Done to In Progress in 4.x Priority Issues Nov 17, 2022
Repository owner moved this from In Progress to Done in 4.x Priority Issues Dec 10, 2022
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.

7 participants