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

TextEdit: Correct typo that broke custom selected font color #30018

Merged
merged 1 commit into from
Jun 24, 2019
Merged

TextEdit: Correct typo that broke custom selected font color #30018

merged 1 commit into from
Jun 24, 2019

Conversation

GlaceGwyneth
Copy link
Contributor

@GlaceGwyneth GlaceGwyneth commented Jun 24, 2019

text_edit's cache was trying to get "font_selected_color" but the colors name as defined in default_theme.cpp is "font_color_selected"
Fixes #27760.

@YeldhamDev
Copy link
Member

Try to use the word fix/fixes/close/closes/resolve/resolves before a issue number so it's tagged automatically for closing when the commit is merged.

@akien-mga akien-mga added this to the 3.2 milestone Jun 24, 2019
@akien-mga
Copy link
Member

Looks good, but there are some more occurrences of trying to set "font_selected_color" which should be fixed too:

$ rg font_selected
scene/gui/text_edit.cpp
1181:                                                           drawer.draw_char(ci, Point2i(char_ofs + char_margin + ofs_x, yofs + ascent), '_', str[j + 1], in_selection && override_selected_font_color ? cache.font_selected_color : color);
1189:                                                           drawer.draw_char(ci, Point2i(char_ofs + char_margin + ofs_x, yofs + ascent), '_', str[j + 1], in_selection && override_selected_font_color ? cache.font_selected_color : color);
1261:                                                   int w = drawer.draw_char(ci, Point2i(char_ofs + char_margin + ofs_x, yofs + ascent), str[j], str[j + 1], in_selection && override_selected_font_color ? cache.font_selected_color : color);
1268:                                                           draw_rect(Rect2(char_ofs + char_margin + ofs_x, yofs + ascent + 2, w, line_width), in_selection && override_selected_font_color ? cache.font_selected_color : color);
1272:                                                   cache.tab_icon->draw(ci, Point2(char_ofs + char_margin + ofs_x, ofs_y + yofs), in_selection && override_selected_font_color ? cache.font_selected_color : color);
1277:                                                   cache.space_icon->draw(ci, Point2(char_ofs + char_margin + ofs_x, ofs_y + yofs), in_selection && override_selected_font_color ? cache.font_selected_color : color);
4557:   cache.font_selected_color = get_color("font_selected_color");

scene/gui/text_edit.h
187:            Color font_selected_color;

editor/plugins/text_editor.cpp
119:    text_edit->add_color_override("font_selected_color", text_selected_color);

editor/plugins/shader_editor_plugin.cpp
127:    get_text_edit()->add_color_override("font_selected_color", text_selected_color);

editor/plugins/script_text_editor.cpp
240:    text_edit->add_color_override("font_selected_color", text_selected_color);

I would suggest to also rename the variables used internally like text_selected_color and cache.font_selected_color to text_color_selected and cache.font_color_selected to avoid the confusion and further errors.

@GlaceGwyneth
Copy link
Contributor Author

I decided not to change text_selected_color because that really is the name of the setting.
Its really just font_color_selected that strangely doesn't end with _color like everything else.
Maybe font_color_selected should be the one changing? What do you think?

@akien-mga
Copy link
Member

Its really just font_color_selected that strangely doesn't end with _color like everything else.
Maybe font_color_selected should be the one changing? What do you think?

Ideally yes, but that would break compatibility with themes made with earlier Godot version, so it can't be done now. We can rename it in 4.0 though, I'll add a mention to it in #16863.

Changes look good, could you squash commits together?

Change several font_selected_color to font_color_selected; the actual name of the override
@akien-mga akien-mga merged commit 338c553 into godotengine:master Jun 24, 2019
@akien-mga
Copy link
Member

Thanks, and congrats for your first Godot PR :)

@GlaceGwyneth GlaceGwyneth deleted the typo-fix-selected-font branch June 24, 2019 10:43
@akien-mga
Copy link
Member

Cherry-picked for 3.1.2.

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.

TextEdit font selected color not work
3 participants