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.

Sign up for GitHub

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

Add a warning when accessing theme prematurely and fix surfaced issues #73475

Merged
merged 4 commits into from
Apr 5, 2023
Merged

Add a warning when accessing theme prematurely and fix surfaced issues #73475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91ff34b
Add a warning when trying to access theme items too early
YuriSizov Apr 3, 2023
db2f99c
Implement theme item cache in TextEdit and CodeEdit
YuriSizov Apr 3, 2023
e401540
Implement theme item cache in ColorPicker and friends
YuriSizov Apr 3, 2023
9b500ab
Fix premature theme item access in editor tools
YuriSizov Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add a warning when trying to access theme items too early
  • Loading branch information
@YuriSizov
YuriSizov committed Apr 3, 2023
commit 91ff34b5b569916479fdcb40430d10cb7f15401c
50 changes: 50 additions & 0 deletions scene/gui/control.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,10 @@ StringName Control::get_theme_type_variation() const {
/// Theme property lookup.

Ref<Texture2D> Control::get_theme_icon(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const Ref<Texture2D> *tex = data.theme_icon_override.getptr(p_name);
if (tex) {
Expand All @@ -2443,6 +2447,10 @@ Ref<Texture2D> Control::get_theme_icon(const StringName &p_name, const StringNam
}

Ref<StyleBox> Control::get_theme_stylebox(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const Ref<StyleBox> *style = data.theme_style_override.getptr(p_name);
if (style) {
Expand All @@ -2462,6 +2470,10 @@ Ref<StyleBox> Control::get_theme_stylebox(const StringName &p_name, const String
}

Ref<Font> Control::get_theme_font(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const Ref<Font> *font = data.theme_font_override.getptr(p_name);
if (font) {
Expand All @@ -2481,6 +2493,10 @@ Ref<Font> Control::get_theme_font(const StringName &p_name, const StringName &p_
}

int Control::get_theme_font_size(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const int *font_size = data.theme_font_size_override.getptr(p_name);
if (font_size && (*font_size) > 0) {
Expand All @@ -2500,6 +2516,10 @@ int Control::get_theme_font_size(const StringName &p_name, const StringName &p_t
}

Color Control::get_theme_color(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const Color *color = data.theme_color_override.getptr(p_name);
if (color) {
Expand All @@ -2519,6 +2539,10 @@ Color Control::get_theme_color(const StringName &p_name, const StringName &p_the
}

int Control::get_theme_constant(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const int *constant = data.theme_constant_override.getptr(p_name);
if (constant) {
Expand All @@ -2538,6 +2562,10 @@ int Control::get_theme_constant(const StringName &p_name, const StringName &p_th
}

bool Control::has_theme_icon(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
if (has_theme_icon_override(p_name)) {
return true;
Expand All @@ -2550,6 +2578,10 @@ bool Control::has_theme_icon(const StringName &p_name, const StringName &p_theme
}

bool Control::has_theme_stylebox(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
if (has_theme_stylebox_override(p_name)) {
return true;
Expand All @@ -2562,6 +2594,10 @@ bool Control::has_theme_stylebox(const StringName &p_name, const StringName &p_t
}

bool Control::has_theme_font(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
if (has_theme_font_override(p_name)) {
return true;
Expand All @@ -2574,6 +2610,10 @@ bool Control::has_theme_font(const StringName &p_name, const StringName &p_theme
}

bool Control::has_theme_font_size(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
if (has_theme_font_size_override(p_name)) {
return true;
Expand All @@ -2586,6 +2626,10 @@ bool Control::has_theme_font_size(const StringName &p_name, const StringName &p_
}

bool Control::has_theme_color(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
if (has_theme_color_override(p_name)) {
return true;
Expand All @@ -2598,6 +2642,10 @@ bool Control::has_theme_color(const StringName &p_name, const StringName &p_them
}

bool Control::has_theme_constant(const StringName &p_name, const StringName &p_theme_type) const {
if (!data.initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
if (has_theme_constant_override(p_name)) {
return true;
Expand Down Expand Up @@ -2894,6 +2942,8 @@ Control *Control::make_custom_tooltip(const String &p_text) const {
void Control::_notification(int p_notification) {
switch (p_notification) {
case NOTIFICATION_POSTINITIALIZE: {
data.initialized = true;

_invalidate_theme_cache();
_update_theme_item_cache();
} break;
Expand Down
2 changes: 2 additions & 0 deletions scene/gui/control.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ class Control : public CanvasItem {

// This Data struct is to avoid namespace pollution in derived classes.
struct Data {
bool initialized = false;

// Global relations.

List<Control *>::Element *RI = nullptr;
Expand Down
50 changes: 50 additions & 0 deletions scene/main/window.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,8 @@ Viewport *Window::get_embedder() const {
void Window::_notification(int p_what) {
switch (p_what) {
case NOTIFICATION_POSTINITIALIZE: {
initialized = true;

_invalidate_theme_cache();
_update_theme_item_cache();
} break;
Expand Down Expand Up @@ -1754,6 +1756,10 @@ StringName Window::get_theme_type_variation() const {
/// Theme property lookup.

Ref<Texture2D> Window::get_theme_icon(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
const Ref<Texture2D> *tex = theme_icon_override.getptr(p_name);
if (tex) {
Expand All @@ -1773,6 +1779,10 @@ Ref<Texture2D> Window::get_theme_icon(const StringName &p_name, const StringName
}

Ref<StyleBox> Window::get_theme_stylebox(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
const Ref<StyleBox> *style = theme_style_override.getptr(p_name);
if (style) {
Expand All @@ -1792,6 +1802,10 @@ Ref<StyleBox> Window::get_theme_stylebox(const StringName &p_name, const StringN
}

Ref<Font> Window::get_theme_font(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
const Ref<Font> *font = theme_font_override.getptr(p_name);
if (font) {
Expand All @@ -1811,6 +1825,10 @@ Ref<Font> Window::get_theme_font(const StringName &p_name, const StringName &p_t
}

int Window::get_theme_font_size(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
const int *font_size = theme_font_size_override.getptr(p_name);
if (font_size && (*font_size) > 0) {
Expand All @@ -1830,6 +1848,10 @@ int Window::get_theme_font_size(const StringName &p_name, const StringName &p_th
}

Color Window::get_theme_color(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
const Color *color = theme_color_override.getptr(p_name);
if (color) {
Expand All @@ -1849,6 +1871,10 @@ Color Window::get_theme_color(const StringName &p_name, const StringName &p_them
}

int Window::get_theme_constant(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
const int *constant = theme_constant_override.getptr(p_name);
if (constant) {
Expand All @@ -1868,6 +1894,10 @@ int Window::get_theme_constant(const StringName &p_name, const StringName &p_the
}

bool Window::has_theme_icon(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
if (has_theme_icon_override(p_name)) {
return true;
Expand All @@ -1880,6 +1910,10 @@ bool Window::has_theme_icon(const StringName &p_name, const StringName &p_theme_
}

bool Window::has_theme_stylebox(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
if (has_theme_stylebox_override(p_name)) {
return true;
Expand All @@ -1892,6 +1926,10 @@ bool Window::has_theme_stylebox(const StringName &p_name, const StringName &p_th
}

bool Window::has_theme_font(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
if (has_theme_font_override(p_name)) {
return true;
Expand All @@ -1904,6 +1942,10 @@ bool Window::has_theme_font(const StringName &p_name, const StringName &p_theme_
}

bool Window::has_theme_font_size(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
if (has_theme_font_size_override(p_name)) {
return true;
Expand All @@ -1916,6 +1958,10 @@ bool Window::has_theme_font_size(const StringName &p_name, const StringName &p_t
}

bool Window::has_theme_color(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
if (has_theme_color_override(p_name)) {
return true;
Expand All @@ -1928,6 +1974,10 @@ bool Window::has_theme_color(const StringName &p_name, const StringName &p_theme
}

bool Window::has_theme_constant(const StringName &p_name, const StringName &p_theme_type) const {
if (!initialized) {
WARN_PRINT_ONCE("Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED");
}

if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == theme_type_variation) {
if (has_theme_constant_override(p_name)) {
return true;
Expand Down
1 change: 1 addition & 0 deletions scene/main/window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class Window : public Viewport {

private:
DisplayServer::WindowID window_id = DisplayServer::INVALID_WINDOW_ID;
bool initialized = false;

String title;
mutable int current_screen = 0;
Expand Down