Godot: TabContainer's tab_changed signal runs every time you click tab

[ivanskodje]

Operating system or device - Godot version:
v2.1.2 (on Steam)

Issue description:
The tab_changed( int tab ) does not only run when you change tab, it also runs when you select a tab regardless if you are changing it. (Video example at 6:06'ish)

Steps to reproduce:

• Create a TabContainer with a few tabs.
• Setup the "tab_changed( int tab ) signal from TabContainer
• Make it print some text when the signal runs
• Play the scene and click the same tab over and over again

Link to minimal example project:
https://mega.nz/#!oQNxCZTJ!k2o2y6I5do4fqKFkaE9abWSRIRY91VD4z1VJxXHQMaw (Project used in the video example)

[RebelliousX]

I don't think this is a bug, in fact, it is advantageous as is. It is the programmer's job to check if current tab has been changed. By comparing current tab number with a local variable in tab container script, initialized at _ready() function. I rely on current behavior.

[bojidar-bg]

Well, actually, it is called tab_changed. not tab_clicked, so it makes sense to have no signal for repeated clicks of the same tab.
@RebelliousX Can you give a concrete example of the "advantageous" part? I see no common uses for this behaviour (but, if there are common uses, we can add another signal without autochecking).

[ivanskodje]

@RebelliousX - If that were true, the signal name would be misleading and should have been changed to tab_selected.

Edit:
(Didn't refresh to see @bojidar-bg 's post)

[RebelliousX]

@bojidar-bg I use it in a multiviewport setting in one of my projects, to reload the scenes without switching tabs.

Besides, I remember not too long ago, I complained about something similar, that if a variable that has a signal (setter/getter), the setter functions will be called even if the value doesn't change.

Suppose there is a setter function for variable x, which currently has the value of 5. If you later say x=5 then the setter function will be called again. It was decided by the majority that it was the intended behavior, which I came to accept.

What I mean by this, if the signal of tab switching is using the same principle of signals for variables, then it shouldn't contradict that.

[bojidar-bg]

One thing is that detecting duplicate tab_changed emmision needs additional bookkeeping in the form of a variable + an if statement + an assignment, while the duplicate set_x call needs just an if statement to check.
Another is that the signal's name implies that the tab changed, while the set X function's name doesn't imply that it can't be called for with a value returned by the get X function.

[RebelliousX]

Well, I have no complains if the name of the function changes.let it be tab_clicked or tab_selected or even tab_set. But functionality wise, it souldn't change IMO.

[hubbyist]

changing current signal's name to "tab_selected" and adding a new signal named "tab_changed" with intended behaviour is a good solution I think.

[akien-mga]

As I mentioned in #7752, I don't think that a tab_selected signal that is only emitted when the current tab is selected makes sense.

IMO, tab_changed should be renamed to tab_selected (and continue emitting a signal regardless of the tab being selected, whether a different one or the current one), and then if there's a use case for it, tab_changed could be readded for when it's a different tab and the actual change is happening (not just the click event).

So basically similar to what #7805 does but without limiting tab_selected to the current tab, that's a bad API.

[RebelliousX]

@akien-mga That would break compatibility with existing code that relies on that signal. The user won't know what went wrong, that is, tab_changed changed to be tab_selected.

BTW, is there a reason why the tab class is almost identical (with maybe some differences) to TabContainer?

[vnen]

Aren't we in a compatibility-breaking phase anyway? I prefer that we use that opportunity to make things more user-friendly. I'm leaned to believe that more people find strange that tab_changed is emitted when the tab is not changed than the ones that rely on such behavior.

[RebelliousX]

Ok, I closed that PR and created another with the changes you suggested #7916