Skip to content

Conversation

Mickeon
Copy link
Member

@Mickeon Mickeon commented Aug 20, 2022

Closes godotengine/godot-proposals#4978.

For Label & RichTextLabel:
percent_visible -> visible_ratio

This way its meaning is much clearer, and now the property can more tidily be organised with the similar visible_characters.
image

Also updates the Documentation of both visible_characters and visible_ratio considerably, to better describe what they do and improve consistency between the two Classes.
image

Also updates the documentation of both `visible_characters` and `visible_ratio` to, better describe what they do and improve consistency between the two Classes.
@Mickeon Mickeon force-pushed the rename-label-visible branch from 91f47d2 to 64bd36e Compare August 26, 2022 10:16
@akien-mga
Copy link
Member

For the record there's also percent_visible in ProgressBar:

$ rg 'percent_visible' -g'!*.po*'
scene/gui/progress_bar.h
39:     bool percent_visible = true;
57:     void set_percent_visible(bool p_visible);
58:     bool is_percent_visible() const;

scene/gui/progress_bar.cpp
44:     if (percent_visible) {
103:                    if (percent_visible) {
128:void ProgressBar::set_percent_visible(bool p_visible) {
129:    if (percent_visible == p_visible) {
132:    percent_visible = p_visible;
137:bool ProgressBar::is_percent_visible() const {
138:    return percent_visible;
144:    ClassDB::bind_method(D_METHOD("set_percent_visible", "visible"), &ProgressBar::set_percent_visible);
145:    ClassDB::bind_method(D_METHOD("is_percent_visible"), &ProgressBar::is_percent_visible);
148:    ADD_PROPERTY(PropertyInfo(Variant::BOOL, "percent_visible"), "set_percent_visible", "is_percent_visible");

doc/classes/ProgressBar.xml
15:             <member name="percent_visible" type="bool" setter="set_percent_visible" getter="is_percent_visible" default="true">
47:                     Font used to draw the fill percentage if [member percent_visible] is [code]true[/code].
50:                     Font size used to draw the fill percentage if [member percent_visible] is [code]true[/code].

editor/plugins/animation_blend_tree_editor_plugin.cpp
236:                    pb->set_percent_visible(false);

It could maybe be renamed too for consistency, though I don't know if visible_ratio is such a good name for it. Maybe fill_ratio since it's about filling a progress bar?

Otherwise this PR seems fine as is.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for the Label and RichTextLabel changes.

@akien-mga akien-mga merged commit c8ef12a into godotengine:master Aug 26, 2022
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the rename-label-visible branch August 26, 2022 13:23
@Mickeon
Copy link
Member Author

Mickeon commented Aug 28, 2022

@akien-mga It could maybe be renamed too for consistency, though I don't know if visible_ratio is such a good name for it. Maybe fill_ratio since it's about filling a progress bar?

Most confusingly, that property is not the visible percentage, it's actually a boolean that when set to true, displays the completion percentage at the center of the ProgressBar.

@akien-mga
Copy link
Member

akien-mga commented Aug 28, 2022

Oh ok, then maybe it could be renamed to show_percentage?

@Mickeon
Copy link
Member Author

Mickeon commented Aug 28, 2022

Sounds pretty good to me, coming soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

Rename RichTextLabel's percent_visible to something else
3 participants