godot icon indicating copy to clipboard operation
godot copied to clipboard

Remove incorrect statement about TextureButton normal texture

Open Meorge opened this issue 1 year ago • 1 comments

The documentation currently states that TextureButtons must have a texture for their "normal" state in texture_normal, but I found this didn't seem to be the case in practice, as noted here: https://github.com/godotengine/godot-docs/issues/9763

This PR simply removes that line about needing the texture from the TextureButton documentation page.

Meorge avatar Aug 16 '24 21:08 Meorge

This depends I'd say what is meant, I think it should instead explain what happens without this texture and suggest using it unless you know what you're doing

AThousandShips avatar Aug 17 '24 09:08 AThousandShips

That makes sense; how does something like this sound?

It is recommended to at least set a texture for the "normal" state ([member texture_normal]). Otherwise, the TextureButton will still receive input events and be clickable, but the user will not be able to see it.

I suppose I should test adding textures for texture_disabled, texture_focused, texture_hover, and texture_pressed to confirm how they interact with texture_normal (or a lack thereof) - from the docs it sounds like these textures may show up once the user performs the relevant interaction, and as such should factor into the description.

Meorge avatar Aug 17 '24 15:08 Meorge

I would suggest you update the PR to add the note you wrote (or similar) at the bottom the leading description. There's nothing explicitly preventing TextureButton from existing without texture_normal. But, if no corresponding texture exists, texture_normal is what the other button states fall back to. So, in a way, it is essential.

The vague statement being removed sucks, anyway. It leaves you with your own thoughts assuming the worst.

Mickeon avatar Dec 02 '24 12:12 Mickeon

Thanks for the reminder! I've updated the PR with a version of what I'd written above. I wasn't entirely sure how to succinctly phrase the idea that the other states fall back to the "normal" state; hopefully the current wording is okay, but if anyone has suggestions for improving it, please feel free to add them in a review!

Meorge avatar Dec 02 '24 14:12 Meorge

I wasn't entirely sure how to succinctly phrase the idea that the other states fall back to the "normal" state

Honestly I feel like that's something that each state texture property's description could be more explicit about, instead of summarising it in the leading description.

Mickeon avatar Dec 02 '24 15:12 Mickeon

I pushed a few changes to elaborate more on how the texture-swapping works, both in the note at the beginning and the descriptions for the texture properties.

Meorge avatar Dec 02 '24 16:12 Meorge

I created a small test project to verify these behaviors before I added the notes to the documentation - a little later today I can post a set of short videos demonstrating them in action

Edit: I've added videos and descriptions of the TextureButton behaviors as textures are assigned and unassigned. Notably, it did lead me to realize that we falsely stated texture_pressed would fall back to texture_normal, when in fact it falls back to texture_hover.

Meorge avatar Dec 02 '24 19:12 Meorge

Is this ready as-is, or do these adjustments still need to be integrated?

Repiteo avatar Dec 12 '24 22:12 Repiteo

Looks like I need to commit those suggestions and squash, and then it should be ready to go. I'll try to get that done within the next hour or two :)

Meorge avatar Dec 12 '24 22:12 Meorge

Updates are pushed and history is squashed!

Meorge avatar Dec 12 '24 23:12 Meorge

Thanks!

Repiteo avatar Dec 13 '24 22:12 Repiteo