godot-proposals icon indicating copy to clipboard operation
godot-proposals copied to clipboard

Add disabled_changed signal to BaseButton, emitted when disabled property changes

Open lostminds opened this issue 1 year ago • 13 comments

Describe the project you are working on

An editor interface with custom Button controls

Describe the problem or limitation you are having in your project

BaseButton has a disabled property that controls if the button is disabled. However, there is no signal that is emitted if this state changes, which makes it a little convoluted to make customized button controls that you want to update correctly based on their disabled state.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Simply add a new signal to BaseButton, for example disabled_changed(new_disabled) that is emitted when this property is changed.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

The same as other signals emitted on property changes in controls.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Not exactly, as there's no good way I know of to listen for changes of a property and you can't override core components setters (like set_disabled() in this case) in your subclass. So you just need to keep track of when you disable your custom buttons and then also call whatever you have set up to also trigger them to update their components to the new disabled state.

Is there a reason why this should be core and not an add-on in the asset library?

BaseButton is a core class and disabled is one of its basic properties.

lostminds avatar Jan 15 '24 17:01 lostminds

When will the button ever be disabled automatically and not by script? Having signals for things that are not automatic is usually not something we do because it is something you can just do yourself, and adding signals is not free

I don't see a reason to add a signal for something that is under user control

AThousandShips avatar Jan 15 '24 17:01 AThousandShips

I don't see a reason to add a signal for something that is under user control

Well, the reason would be to ensure consistency and make the code simpler. Like using a setter function to trigger something you always want to trigger when the variable changes. So that when disabling a control you only need to write custom_button.disabled = true instead of

custom_button.disabled = true
custom_button.updateBasedOnDisableState()

lostminds avatar Jan 15 '24 17:01 lostminds

But you can do that, so there's no reason to add overhead for everyone else but adding a signal when you can do it yourself, we don't do these things elsewhere barely ever if at all

AThousandShips avatar Jan 15 '24 17:01 AThousandShips

Well, I guess you're right that it's a bit of a roundabout way of doing this to add a new signal. Since what I'd really want is to override the set_disabled setter in the script to be able to catch the change in my subclass of Button, keeping this neat and tidy in the class. But as I understand it inheritance works a little differently in Godot so overriding core classes like this can't really be done, and this is why I'm looking for some other way of doing this. If adding a signal is not a good way of solving it, perhaps there is some other better way?

lostminds avatar Jan 15 '24 19:01 lostminds

But you're in full control, just handle where you update this, there's no situation where the change isn't in your control, so just write the places where you update this so it does what you want, that's the power you have with scripts

There's no other things that randomly change this beyond your control, right?

AThousandShips avatar Jan 15 '24 19:01 AThousandShips

If you don't mind attaching scripts to your buttons, you can do:

extends BaseButton

var disabled2: bool:
	set(d):
		disabled = d
		some update...
	get:
		return disabled

and set disabled2 instead of disabled.

KoBeWi avatar Jan 15 '24 20:01 KoBeWi

and set disabled2 instead of disabled.

Yes, thank you, that's certainly an alternative that at least saves a line of code. But it misses the central reason behind the proposal, to be able to make easily reusable modular subclassed control classes and avoid coding mistakes: Basically to be able to make a CustomButton class that can then be used in scripts in the same way as a regular Button (Or a custom Label in https://github.com/godotengine/godot-proposals/issues/8790 with similar reasoning). To avoid the risk of making mistakes by not knowing that for this type of Button you can't use the regular disabled and instead need to set disabled2. Or cases where the script just defines the var as a Button (when it's actually a reference to a CustomButton class button).

Since the base class in this case already has a property disabled that is controls this behavior it makes sense to me to let this be the property that controls the behavior in a subclass of the control as well. And this then leads to the need to somehow know in the subclass script when the base class disabled changes to achieve this consistently.

lostminds avatar Jan 16 '24 07:01 lostminds

The simplest way to handle any property changes which lead to a different visual appearance is to use the NOTIFICATION_DRAW notification. Anything which calls queue_redraw() can be detected this way.

YuriSizov avatar Jan 16 '24 11:01 YuriSizov

But you're in full control, just handle where you update this, there's no situation where the change isn't in your control, so just write the places where you update this so it does what you want, that's the power you have with scripts

There's no other things that randomly change this beyond your control, right?

That's missing the point entirely, because the responsibility of "I need to remember to emit the signal" now rests with the user, and not the implementation. In other words, every single place this new control is used is a bug waiting to happen, and the customised control has no way to behave the same as the base button it's extending.

This is a pervasive problem in GDScript; there's simply no way to extend a core class seamlessly if the extension class wants to react to or change something the core does.

mathrick avatar Jul 02 '24 00:07 mathrick

I also expected some kind of signal. In my button component, I created a new function _set_disabled(value) and the first thing that I do is set the disabled parameter. Then, I execute my scripts.

Thats the way that I found to set disabled and then do some logic. Now, instead of using my_component.disabled I will use my_component._set_disabled(true/false)

It worked :)

mfventura avatar Jul 19 '24 17:07 mfventura

+1 for the necessity of the described signal.

As styling a button is limited via the theme system (e.g. "focused" is not a real visual state, as it adds an overlay, but there's no way to also set the background texture all the other states use; also, if a texture in a style requires a different filtering, it's also not possible in a theme/style based approach), it can be necessary to define a custom scene that handles the visuals with custom nodes and scripts. Having a Button node as root for such custom scene is useful, as the button's text can be set conveniently and also all of a Button's regular signals and properties remain available. However, without a way to either overload the set_disabled function or to observe a disabled_changed signal, handling a button's disabled state requires extra effort.

sszigeti avatar Feb 10 '25 20:02 sszigeti

I made a plugin to expose godot UI controls to the browser accessibility tree, this works by creating hidden DOM elements and including the equivalent aria-* attributes of the node control properties ( visible, disabled, checked, etc.. ), this works well for setting the initial value but to keep things in sync you have to manually call and update function that can clutter the UI code of your game. Sharing this as another use case for having a way to observe changes for disabled and other built in properties.

btzr-io avatar Mar 01 '25 19:03 btzr-io

The simplest way to handle any property changes that lead to a different visual appearance is to use the NOTIFICATION_DRAW notification. Anything which calls queue_redraw() can be detected this way.

I think I've found a not-too-horrible way to have a standalone component that observes a plain Button, and reacts to its visual changes without using polling, leveraging the Button's draw signal, as per @YuriSizov's recommendation.

For this demonstration I recommend using a MarginContainer with a Panel inside.

Remember to set both nodes' mouse_filter to MOUSE_FILTER_IGNORE, otherwise these nodes will block your button, despite their purpose being purely aesthetic.

The scene's root node could have a script like so:

extends MarginContainer
class_name ButtonBorder

@export var default_color := Color("4d4739")
@export var pressed_color := Color("fff")
@export var hover_color := Color("9b9072")
@export var disabled_color := Color("9c917300")
@export_category("Local nodes")
@export var visuals:Control # Remember to assign the Panel node inside this MarginContainer in the Godot Inspector.

var button:Button


func UpdateVisuals() -> void:
	visuals.self_modulate = disabled_color if button.disabled else pressed_color if button.button_pressed else hover_color if button.is_hovered() else default_color


func _ready() ->  void:
	button = get_parent()
	button.draw.connect(UpdateVisuals)
	UpdateVisuals()

Then, you'd merely need to add an instance of this scene to any button, and this would activate only when the button is actually redrawn -- without having to modify the Button's script. This scene also works with any Button-derived scene.

sszigeti avatar Mar 15 '25 16:03 sszigeti