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

TabContainer not set up properly when its children are ready #95819

Open
Grandro opened this issue Aug 19, 2024 · 5 comments
Open

TabContainer not set up properly when its children are ready #95819

Grandro opened this issue Aug 19, 2024 · 5 comments

Comments

@Grandro
Copy link

Grandro commented Aug 19, 2024

Tested versions

v4.3.stable.official [77dcf97]
Works fine in v4.2.2 (and before)

System information

Windows 10

Issue description

When instantiating TabContainer children via code the TabContainer is not set up properly when one of the children is ready. Compare these two outputs:
v4.2.2
image

v4.3
image
This forces users to call_deferred any logic that relies on these properties, which is pretty inconvenient.

Steps to reproduce

  1. Instantiate a TabContainer
  2. Instantiate a Control node as a child of the TabContainer
  3. Connect the ready signal of the child
  4. Check state of the TabContainer within the connected function

Minimal reproduction project (MRP)

TabContainerCurrentTab.zip
This is a 4.2.2 project which can easily be converted to 4.3

@matheusmdx
Copy link
Contributor

Bisecting points to #87194 as the culprit:

image

@kitbdev
Copy link
Contributor

kitbdev commented Aug 20, 2024

This is just because the default changed in 4.3, from 0 to -1.
The child needs to become ready before the TabContainer can handle it, this is true in 4.2 as well. The child's ready notification is not a good place to use its parent TabContainer's methods. (get_current_tab_control gets the control on demand which is why it works, but stuff like get_tab_count will not see this newest child).
If you print immediately after the add_child call, you can see that the current tab is set immediately and doesn't need to wait a frame.

@Grandro
Copy link
Author

Grandro commented Aug 20, 2024

@kitbdev You are right, the state of the TabContainer in 4.3 and 4.2.2 is not really different. current_tab as you said just defaults to -1 now instead of 0. If I print get_tab_count() both in 4.2.2 and 4.3 0 is being printed, so 4.2.2 is actually the inconsistent one with get_current_tab_control() not being null while get_tab_count() is 0.

The problem is from the perspective of the user I think it is a fair assumption that a TabContainer should be useable when its children are ready. Else you run into this dilemma where you have to filter out the case where a child emits a signal you want to react to, but cant because the TabContainer is not set up properly yet. Workarounds are possible, but not very convenient.

I'll leave this issue open for the responsible people to decide whether the current behaviour has to be changed/can be changed or if this is how it should be.

@Grandro Grandro changed the title Regression: TabContainer takes a frame too long to set up in 4.3 TabContainer not set up properly when its children are ready Aug 20, 2024
@Maran23
Copy link
Contributor

Maran23 commented Sep 4, 2024

Probably caused by call_deferred in the Godot code. I really dont understand why this method is used that often.
Deferring something should only be used when needed, e.g. there must be a good reason (queueing something in, e.g. drawing, focus something later, ...).

Having said that, I agree that this is probably not intended to work.
I think the node is added first, ready is emitted and after that the tab_container will react to the change (add_child_notify).

@WhalesState
Copy link
Contributor

In GDScript, it's easier to deal with this, since you can use await get_tree().process_frame to solve many issues. However, sometimes you need to use it twice in a row, possibly because a deferred method is called somewhere after using call_deferred(), which makes it harder to solve this issue in C++.

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

No branches or pull requests

6 participants