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

Rework the Configuration Warning system #90049

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 28 additions & 7 deletions core/io/resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,6 @@ String Resource::get_name() const {
return name;
}

void Resource::update_configuration_warning() {
if (_update_configuration_warning) {
_update_configuration_warning();
}
}

bool Resource::editor_can_reload_from_file() {
return true; //by default yes
}
Expand Down Expand Up @@ -492,7 +486,6 @@ void Resource::reset_local_to_scene() {
}

Node *(*Resource::_get_local_scene_func)() = nullptr;
void (*Resource::_update_configuration_warning)() = nullptr;

void Resource::set_as_translation_remapped(bool p_remapped) {
if (remapped_list.in_list() == p_remapped) {
Expand Down Expand Up @@ -539,6 +532,32 @@ String Resource::get_id_for_path(const String &p_path) const {
#endif
}

#ifdef TOOLS_ENABLED
Vector<ConfigurationInfo> Resource::get_configuration_info() const {
Vector<ConfigurationInfo> ret;

Array info;
if (GDVIRTUAL_CALL(_get_configuration_info, info)) {
ret.resize(info.size());

ConfigurationInfo *ptrw = ret.ptrw();
for (const Variant &variant : info) {
*ptrw++ = ConfigurationInfo::from_variant(variant);
}
}

return ret;
}
#endif

void Resource::update_configuration_info() {
#ifdef TOOLS_ENABLED
if (ConfigurationInfo::configuration_info_changed_func) {
ConfigurationInfo::configuration_info_changed_func(this);
}
#endif
}

void Resource::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_path", "path"), &Resource::_set_path);
ClassDB::bind_method(D_METHOD("take_over_path", "path"), &Resource::_take_over_path);
Expand All @@ -561,6 +580,7 @@ void Resource::_bind_methods() {
ClassDB::bind_static_method("Resource", D_METHOD("generate_scene_unique_id"), &Resource::generate_scene_unique_id);
ClassDB::bind_method(D_METHOD("set_scene_unique_id", "id"), &Resource::set_scene_unique_id);
ClassDB::bind_method(D_METHOD("get_scene_unique_id"), &Resource::get_scene_unique_id);
ClassDB::bind_method(D_METHOD("update_configuration_info"), &Resource::update_configuration_info);

ClassDB::bind_method(D_METHOD("emit_changed"), &Resource::emit_changed);

Expand All @@ -578,6 +598,7 @@ void Resource::_bind_methods() {
GDVIRTUAL_BIND(_get_rid);
GDVIRTUAL_BIND(_reset_state);
GDVIRTUAL_BIND(_set_path_cache, "path");
GDVIRTUAL_BIND(_get_configuration_info);
}

Resource::Resource() :
Expand Down
10 changes: 8 additions & 2 deletions core/io/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

#include "core/io/resource_uid.h"
#include "core/object/class_db.h"
#include "core/object/configuration_info.h"
#include "core/object/gdvirtual.gen.inc"
#include "core/object/ref_counted.h"
#include "core/templates/safe_refcount.h"
Expand Down Expand Up @@ -86,6 +87,7 @@ class Resource : public RefCounted {

virtual void reset_local_to_scene();
GDVIRTUAL0(_setup_local_to_scene);
GDVIRTUAL0RC(Array, _get_configuration_info);

GDVIRTUAL0RC(RID, _get_rid);

Expand All @@ -94,9 +96,7 @@ class Resource : public RefCounted {

public:
static Node *(*_get_local_scene_func)(); //used by editor
static void (*_update_configuration_warning)(); //used by editor

void update_configuration_warning();
virtual bool editor_can_reload_from_file();
virtual void reset_state(); //for resources that use variable amount of properties, either via _validate_property or _get_property_list, this function needs to be implemented to correctly clear state
virtual Error copy_from(const Ref<Resource> &p_resource);
Expand Down Expand Up @@ -152,6 +152,12 @@ class Resource : public RefCounted {
void set_id_for_path(const String &p_path, const String &p_id);
String get_id_for_path(const String &p_path) const;

#ifdef TOOLS_ENABLED
virtual Vector<ConfigurationInfo> get_configuration_info() const;
#endif

void update_configuration_info();

Resource();
~Resource();
};
Expand Down
112 changes: 112 additions & 0 deletions core/object/configuration_info.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**************************************************************************/
/* configuration_info.cpp */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#include "core/object/callable_method_pointer.h"
RedMser marked this conversation as resolved.
Show resolved Hide resolved

#include "configuration_info.h"

#ifdef TOOLS_ENABLED

// Registered in editor, to avoid tight coupling.
void (*ConfigurationInfo::configuration_info_changed_func)(Object *p_object) = nullptr;

void ConfigurationInfo::queue_error_print(const String &p_error) {
if (queued_errors_to_print.is_empty()) {
callable_mp_static(&ConfigurationInfo::_print_errors_from_queue).call_deferred();
}
queued_errors_to_print.insert(p_error);
}

void ConfigurationInfo::_print_errors_from_queue() {
for (const String &error : queued_errors_to_print) {
ERR_PRINT(error);
}
queued_errors_to_print.clear();
}

ConfigurationInfo ConfigurationInfo::from_variant(const Variant &p_variant) {
if (p_variant.get_type() == Variant::STRING) {
return ConfigurationInfo(String(p_variant));
} else if (p_variant.get_type() == Variant::DICTIONARY) {
return ConfigurationInfo(Dictionary(p_variant));
} else {
queue_error_print("Attempted to convert a ConfigurationInfo which is neither a string nor a dictionary, but a " + Variant::get_type_name(p_variant.get_type()));
return ConfigurationInfo();
}
}

ConfigurationInfo::Severity ConfigurationInfo::string_to_severity(const String &p_severity) {
if (p_severity == "error") {
return ConfigurationInfo::Severity::ERROR;
} else if (p_severity == "warning") {
return ConfigurationInfo::Severity::WARNING;
} else if (p_severity == "info") {
return ConfigurationInfo::Severity::INFO;
} else {
queue_error_print("Severity of Configuration Info must be one of \"error\", \"warning\" or \"info\", received \"" + p_severity + "\".");
return ConfigurationInfo::Severity::NONE;
}
}

bool ConfigurationInfo::ensure_valid(Object *p_owner) const {
if (message.is_empty()) {
queue_error_print("Configuration Info may not have an empty message.");
return false;
}

if (p_owner != nullptr && !property_name.is_empty()) {
bool has_property = false;
p_owner->get(property_name, &has_property);
if (!has_property) {
queue_error_print(vformat("Configuration Info on %s refers to property \"%s\" that does not exist.", p_owner->get_class_name(), property_name));
return false;
}
}

return true;
}

ConfigurationInfo::ConfigurationInfo() :
ConfigurationInfo(String()) {
}

ConfigurationInfo::ConfigurationInfo(const Dictionary &p_dict) {
message = p_dict.get("message", "");
property_name = p_dict.get("property", StringName());
severity = string_to_severity(p_dict.get("severity", "warning"));
}

ConfigurationInfo::ConfigurationInfo(const String &p_message, const StringName &p_property_name, Severity p_severity) {
message = p_message;
property_name = p_property_name;
severity = p_severity;
}

#endif // TOOLS_ENABLED
90 changes: 90 additions & 0 deletions core/object/configuration_info.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/**************************************************************************/
/* configuration_info.h */
/**************************************************************************/
/* This file is part of: */
/* GODOT ENGINE */
/* https://godotengine.org */
/**************************************************************************/
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
/* */
/* Permission is hereby granted, free of charge, to any person obtaining */
/* a copy of this software and associated documentation files (the */
/* "Software"), to deal in the Software without restriction, including */
/* without limitation the rights to use, copy, modify, merge, publish, */
/* distribute, sublicense, and/or sell copies of the Software, and to */
/* permit persons to whom the Software is furnished to do so, subject to */
/* the following conditions: */
/* */
/* The above copyright notice and this permission notice shall be */
/* included in all copies or substantial portions of the Software. */
/* */
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#ifndef CONFIGURATION_INFO_H
#define CONFIGURATION_INFO_H

#ifdef TOOLS_ENABLED

#include "core/templates/hash_set.h"
#include "core/variant/variant.h"

#ifdef ERROR
#undef ERROR // Define from Windows APIs
#endif

#define CONFIG_WARNING(message) infos.push_back(ConfigurationInfo(message, "", ConfigurationInfo::Severity::WARNING));
#define CONFIG_WARNING_P(message, property_name) infos.push_back(ConfigurationInfo(message, property_name, ConfigurationInfo::Severity::WARNING));

class ConfigurationInfo {
public:
enum class Severity {
INFO,
WARNING,
ERROR,
MAX,
NONE = -1,
};

private:
String message;
StringName property_name;
Severity severity;

inline static HashSet<String> queued_errors_to_print;

static void queue_error_print(const String &p_error);
static void _print_errors_from_queue();

public:
static void (*configuration_info_changed_func)(Object *p_object);

static ConfigurationInfo from_variant(const Variant &p_variant);
static Severity string_to_severity(const String &p_severity);

bool ensure_valid(Object *p_owner) const;
String get_message() const { return message; }
StringName get_property_name() const { return property_name; }
Severity get_severity() const { return severity; }

bool operator==(const ConfigurationInfo &p_val) const {
return (message == p_val.message) &&
(property_name == p_val.property_name) &&
(severity == p_val.severity);
}

ConfigurationInfo();
ConfigurationInfo(const Dictionary &p_dict);
ConfigurationInfo(const String &p_message, const StringName &p_property_name = StringName(), Severity p_severity = Severity::WARNING);
};

#endif // TOOLS_ENABLED

#endif // CONFIGURATION_INFO_H
42 changes: 40 additions & 2 deletions doc/classes/Node.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,39 @@
Corresponds to the [constant NOTIFICATION_EXIT_TREE] notification in [method Object._notification] and signal [signal tree_exiting]. To get notified when the node has already left the active tree, connect to the [signal tree_exited].
</description>
</method>
<method name="_get_configuration_warnings" qualifiers="virtual const">
<method name="_get_configuration_info" qualifiers="virtual const">
<return type="Array" />
<description>
Override this method to inform the user about the configuration of this node.
Call [method update_configuration_info] when the messages need to be updated for this node.
The elements in the array returned from this method are displayed at the top of the Inspector dock, as well as an icon in the Scene dock. The script that overrides it must be a [code]tool[/code] script.
Each array element must either be a [String] or a [Dictionary].
A dictionary element must contain a key [code]message[/code] of type [String] which is shown in the user interface. Following optional keys are supported:
- [code]property[/code] of type [StringName], which adds an icon next to the property with the same name.
- [code]severity[/code] can be set to [code]"error"[/code], [code]"warning"[/code], or [code]"info"[/code] to change the icon and text color. Defaults to warning.
If a string is found in the returned array, it is treated as a warning message with no property set.
Returning an empty array produces no messages.
[codeblock]
@tool
extends Node

@export var energy = 0:
set(value):
energy = value
update_configuration_info()

func _get_configuration_info():
if energy &lt; 0:
return [{
"property": "energy",
"message": "Energy must be 0 or greater."
}]
else:
return []
[/codeblock]
</description>
</method>
<method name="_get_configuration_warnings" qualifiers="virtual const" deprecated="Use [method _get_configuration_info] instead.">
<return type="PackedStringArray" />
<description>
The elements in the array returned from this method are displayed as warnings in the Scene dock if the script that overrides it is a [code]tool[/code] script.
Expand Down Expand Up @@ -982,7 +1014,13 @@
This is the default behavior for all nodes. Calling [method Object.set_translation_domain] disables this behavior.
</description>
</method>
<method name="update_configuration_warnings">
<method name="update_configuration_info">
<return type="void" />
<description>
Refreshes the configuration information displayed for this node in the Scene dock and Inspector dock. Use [method _get_configuration_info] to customize the messages to display.
</description>
</method>
<method name="update_configuration_warnings" deprecated="Use [method update_configuration_info] instead.">
<return type="void" />
<description>
Refreshes the warnings displayed for this node in the Scene dock. Use [method _get_configuration_warnings] to customize the warning messages to display.
Expand Down
Loading