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

Made tooltip on TileSet Paint Button give more information when disabled #86947

Merged

Conversation

TheSofox
Copy link
Contributor

@TheSofox TheSofox commented Jan 8, 2024

Fixes #78162

When TileSet becomes read only (such as multiple scenes using it at the same time), the Paint Button becomes disabled. Apparently this behaviour is not properly communicated to users. This PR changes the tooltip of the Paint Button when enabled/disabled to communicate this behaviour better to the user. When enabled the tooltip is the normal "Paint properties.", however now when disabled the tooltip is "TileSet is in read-only mode. Make the resource unique to edit TileSet properties."

@groud
Copy link
Member

groud commented Jan 8, 2024

Hey, thanks for the contribution!

I think the idea is quite good, and the implementation is acceptable, but it is a bit unusual to define strings as variables like that. It makes things a bit more difficult to track IMO. I think I would suggest, instead doing it that way, to create some sort of an update_readonly() method, and update buttons/tooltips/etc... there.
That would avoid having to define those strings elsewhere.

@TheSofox
Copy link
Contributor Author

TheSofox commented Jan 8, 2024

Good feedback, I've implemented it by adding a _update_buttons_enabled() function and removing the string variables.

@TheSofox TheSofox force-pushed the tile-set-paint-button-tooltip branch from db29012 to 3614aa9 Compare January 9, 2024 15:09
@TheSofox
Copy link
Contributor Author

TheSofox commented Jan 9, 2024

Renamed function _update_buttons() since it just seemed neater.
The function also contains other bits of repeated code, so I think it cleans up the code overall.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jun 4, 2024
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga merged commit 92ea322 into godotengine:master Jun 4, 2024
15 checks passed
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.

TileSet "Paint" button for painting properties stops working and becomes greyed for an unknown reason
3 participants