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

Fix error spam when adding tabs to TabBar without deselect #88494

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Feb 18, 2024

Ensures current has a valid value after resizing if deselection is not allowed.

The revert marker being visible for the deselect disabled case is a bit annoying and confusing but is a bit more complex to solve, and unsure how it would affect other things with the default values etc., but worth looking into in the future as well.

MRP:
TabMRP.zip

Note how it will print:

ERROR: Index p_idx = -1 is out of bounds (tabs.size() = 2).
   at: TabBar::ensure_tab_visible (scene/gui/tab_bar.cpp:1555)

When opening the file, and also when resizing the tab bar

This ensures the current value is updated to the correct range in this case

Edit: Confirmed that this works on top of:

@AThousandShips AThousandShips added this to the 4.3 milestone Feb 18, 2024
@AThousandShips AThousandShips requested a review from a team as a code owner February 18, 2024 13:29
scene/gui/tab_bar.cpp Outdated Show resolved Hide resolved
@kitbdev
Copy link
Contributor

kitbdev commented Feb 20, 2024

Works good for changing the tab amount.

The issue can still happen when toggling the tab's disabled property.
Set them all to disabled, set the current to -1, then enable one of the tabs, and the error will occur.
To fix this, a similar change should be made in TabBar::set_tab_disabled().

And it can't be set in the inspector, but we might want to do the same in TabBar::set_tab_hidden() since _can_deselect() also checks that.

@AThousandShips
Copy link
Member Author

Will fix that later today, thank you!

@AThousandShips
Copy link
Member Author

Removed the check for _can_deselect from the ensure_tab_visible, this solves the above errors as well, it doesn't make sense to have that check there as ensuring the visibility of the deselected tab doesn't mean anything regardless of the deselection state

scene/gui/tab_bar.cpp Outdated Show resolved Hide resolved
`current` was allowed to be `-1` when deselection was disabled, causing
errors in other methods when updating the size.
@akien-mga akien-mga merged commit 0a3f162 into godotengine:master Feb 22, 2024
16 checks passed
@AThousandShips AThousandShips deleted the tab_fix branch February 22, 2024 10:25
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

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

Successfully merging this pull request may close these issues.

4 participants