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

Switching scene tabs with many tabs may hide tabs #81513

Closed
KoBeWi opened this issue Sep 10, 2023 · 3 comments · Fixed by #83957
Closed

Switching scene tabs with many tabs may hide tabs #81513

KoBeWi opened this issue Sep 10, 2023 · 3 comments · Fixed by #83957

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Sep 10, 2023

Godot version

4.2 dev4

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (NVIDIA; 30.0.15.1403) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

When you have many tabs opened and they are overflowing with the < > buttons, when you switch scene tab it may hide some tabs in specific circumstances:

godot_gY3WZuyXgq.mp4

I'd expect that simply switching to another tab has no effect on visibility.

Steps to reproduce

  1. Open many scene tabs
  2. Repeat what I did on the video
    May depend on tab names and window size.

Minimal reproduction project

N/A

@jsjtxietian
Copy link
Contributor

I digged into the code a little bit, it seems everytime we click a scene, godot will clear all the scene tabs with clear_tabs and recreate them one by one, then call scene_tabs->set_current_tab to set the clicked scene as current tab.

So it seems like scene tab did not remember the state it last called ? clear_tabs effects offset and make it 0.

I think maybe we can make a special case for this and don't recreate everything when user is just switching scene tabs.

@YuriSizov
Copy link
Contributor

I think maybe we can make a special case for this and don't recreate everything when user is just switching scene tabs.

Well, there is an old code comment in there that highlights that the current approach is awful and we ought to fix it. So no need to special case it, it should be a straight fix 🙂

@jsjtxietian
Copy link
Contributor

Aha I saw that comment, but changing the current approach will definitely need more test, I can give it a try :)

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.

4 participants