Skip to content

Commit

Permalink
-Moved EditorDefaultValue to ClassDB, made it core
Browse files Browse the repository at this point in the history 
-Removed one and zero hints for properties, replaced by default value
  • Loading branch information
@reduz
reduz committed Nov 8, 2018
1 parent e7cb47e commit f2e5405
Show file tree
Hide file tree
Showing 53 changed files with 303 additions and 323 deletions.
8 changes: 4 additions & 4 deletions core/bind/core_bind.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2866,10 +2866,10 @@ void JSONParseResult::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_error_line", "error_line"), &JSONParseResult::set_error_line);
ClassDB::bind_method(D_METHOD("set_result", "result"), &JSONParseResult::set_result);

ADD_PROPERTYNZ(PropertyInfo(Variant::OBJECT, "error", PROPERTY_HINT_NONE, "Error", PROPERTY_USAGE_CLASS_IS_ENUM), "set_error", "get_error");
ADD_PROPERTYNZ(PropertyInfo(Variant::STRING, "error_string"), "set_error_string", "get_error_string");
ADD_PROPERTYNZ(PropertyInfo(Variant::INT, "error_line"), "set_error_line", "get_error_line");
ADD_PROPERTYNZ(PropertyInfo(Variant::NIL, "result", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NIL_IS_VARIANT), "set_result", "get_result");
ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "error", PROPERTY_HINT_NONE, "Error", PROPERTY_USAGE_CLASS_IS_ENUM), "set_error", "get_error");
ADD_PROPERTY(PropertyInfo(Variant::STRING, "error_string"), "set_error_string", "get_error_string");
ADD_PROPERTY(PropertyInfo(Variant::INT, "error_line"), "set_error_line", "get_error_line");
ADD_PROPERTY(PropertyInfo(Variant::NIL, "result", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NIL_IS_VARIANT), "set_result", "get_result");
}

void JSONParseResult::set_error(Error p_error) {
Expand Down
36 changes: 36 additions & 0 deletions core/class_db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,41 @@ void ClassDB::get_extensions_for_type(const StringName &p_class, List<String> *p
}
}

HashMap<StringName, HashMap<StringName, Variant> > ClassDB::default_values;

Variant ClassDB::class_get_default_property_value(const StringName &p_class, const StringName &p_property) {

if (!default_values.has(p_class)) {

default_values[p_class] = HashMap<StringName, Variant>();

if (ClassDB::can_instance(p_class)) {

Object *c = ClassDB::instance(p_class);
List<PropertyInfo> plist;
c->get_property_list(&plist);
for (List<PropertyInfo>::Element *E = plist.front(); E; E = E->next()) {
if (E->get().usage & (PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR)) {

Variant v = c->get(E->get().name);
default_values[p_class][E->get().name] = v;
}
}
memdelete(c);
}
}

if (!default_values.has(p_class)) {
return Variant();
}

if (!default_values[p_class].has(p_property)) {
return Variant();
}

return default_values[p_class][p_property];
}

RWLock *ClassDB::lock = NULL;

void ClassDB::init() {
Expand All @@ -1393,6 +1428,7 @@ void ClassDB::cleanup() {
classes.clear();
resource_base_extensions.clear();
compat_classes.clear();
default_values.clear();

memdelete(lock);
}
Expand Down
4 changes: 4 additions & 0 deletions core/class_db.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ class ClassDB {

static void _add_class2(const StringName &p_class, const StringName &p_inherits);

static HashMap<StringName, HashMap<StringName, Variant> > default_values;

public:
// DO NOT USE THIS!!!!!! NEEDS TO BE PUBLIC BUT DO NOT USE NO MATTER WHAT!!!
template <class T>
Expand Down Expand Up @@ -352,6 +354,8 @@ class ClassDB {
static void get_enum_list(const StringName &p_class, List<StringName> *p_enums, bool p_no_inheritance = false);
static void get_enum_constants(const StringName &p_class, const StringName &p_enum, List<StringName> *p_constants, bool p_no_inheritance = false);

static Variant class_get_default_property_value(const StringName &p_class, const StringName &p_property);

static StringName get_category(const StringName &p_node);

static void set_class_enabled(StringName p_class, bool p_enable);
Expand Down
5 changes: 3 additions & 2 deletions core/global_constants.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,8 +547,9 @@ void register_global_constants() {
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_INTERNATIONALIZED);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_GROUP);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_CATEGORY);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_STORE_IF_NONZERO);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_STORE_IF_NONONE);
//deprecated, replaced by ClassDB function to check default value
//BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_STORE_IF_NONZERO);
//BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_STORE_IF_NONONE);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_NO_INSTANCE_STATE);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_RESTART_IF_CHANGED);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_USAGE_SCRIPT_VARIABLE);
Expand Down
7 changes: 6 additions & 1 deletion core/io/resource_format_binary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1813,8 +1813,13 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const RES &p
Property p;
p.name_idx = get_string_index(F->get().name);
p.value = E->get()->get(F->get().name);
if (((F->get().usage & PROPERTY_USAGE_STORE_IF_NONZERO) && p.value.is_zero()) || ((F->get().usage & PROPERTY_USAGE_STORE_IF_NONONE) && p.value.is_one()))

Variant default_value = ClassDB::class_get_default_property_value(E->get()->get_class(), F->get().name);

if (default_value.get_type() != Variant::NIL && bool(Variant::evaluate(Variant::OP_EQUAL, p.value, default_value))) {
continue;
}

p.pi = F->get();

rd.properties.push_back(p);
Expand Down
7 changes: 4 additions & 3 deletions core/object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -636,10 +636,11 @@ void Object::get_property_list(List<PropertyInfo> *p_list, bool p_reversed) cons
#ifdef TOOLS_ENABLED
p_list->push_back(PropertyInfo(Variant::NIL, "Script", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_GROUP));
#endif
p_list->push_back(PropertyInfo(Variant::OBJECT, "script", PROPERTY_HINT_RESOURCE_TYPE, "Script", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_STORE_IF_NONZERO));
p_list->push_back(PropertyInfo(Variant::OBJECT, "script", PROPERTY_HINT_RESOURCE_TYPE, "Script", PROPERTY_USAGE_DEFAULT));
}
if (!metadata.empty()) {
p_list->push_back(PropertyInfo(Variant::DICTIONARY, "__meta__", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NOEDITOR | PROPERTY_USAGE_INTERNAL));
}
if (!metadata.empty())
p_list->push_back(PropertyInfo(Variant::DICTIONARY, "__meta__", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NOEDITOR | PROPERTY_USAGE_INTERNAL | PROPERTY_USAGE_STORE_IF_NONZERO));
if (script_instance && !p_reversed) {
p_list->push_back(PropertyInfo(Variant::NIL, "Script Variables", PROPERTY_HINT_NONE, String(), PROPERTY_USAGE_CATEGORY));
script_instance->get_property_list(p_list);
Expand Down
9 changes: 3 additions & 6 deletions core/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ enum PropertyUsageFlags {
PROPERTY_USAGE_INTERNATIONALIZED = 64, //hint for internationalized strings
PROPERTY_USAGE_GROUP = 128, //used for grouping props in the editor
PROPERTY_USAGE_CATEGORY = 256,
PROPERTY_USAGE_STORE_IF_NONZERO = 512, //only store if nonzero
PROPERTY_USAGE_STORE_IF_NONONE = 1024, //only store if false
//those below are deprecated thanks to ClassDB's now class value cache
//PROPERTY_USAGE_STORE_IF_NONZERO = 512, //only store if nonzero
//PROPERTY_USAGE_STORE_IF_NONONE = 1024, //only store if false

This comment has been minimized.

Sign in to view

Copy link
@neikeq

neikeq Nov 8, 2018

Contributor

If these are deprecated, shouldn't it be kept until 4.0?
PROPERTY_USAGE_NO_INSTANCE_STATE = 2048,
PROPERTY_USAGE_RESTART_IF_CHANGED = 4096,
PROPERTY_USAGE_SCRIPT_VARIABLE = 8192,
Expand All @@ -126,10 +127,6 @@ enum PropertyUsageFlags {
#define ADD_SIGNAL(m_signal) ClassDB::add_signal(get_class_static(), m_signal)
#define ADD_PROPERTY(m_property, m_setter, m_getter) ClassDB::add_property(get_class_static(), m_property, _scs_create(m_setter), _scs_create(m_getter))
#define ADD_PROPERTYI(m_property, m_setter, m_getter, m_index) ClassDB::add_property(get_class_static(), m_property, _scs_create(m_setter), _scs_create(m_getter), m_index)
#define ADD_PROPERTYNZ(m_property, m_setter, m_getter) ClassDB::add_property(get_class_static(), (m_property).added_usage(PROPERTY_USAGE_STORE_IF_NONZERO), _scs_create(m_setter), _scs_create(m_getter))
#define ADD_PROPERTYINZ(m_property, m_setter, m_getter, m_index) ClassDB::add_property(get_class_static(), (m_property).added_usage(PROPERTY_USAGE_STORE_IF_NONZERO), _scs_create(m_setter), _scs_create(m_getter), m_index)
#define ADD_PROPERTYNO(m_property, m_setter, m_getter) ClassDB::add_property(get_class_static(), (m_property).added_usage(PROPERTY_USAGE_STORE_IF_NONONE), _scs_create(m_setter), _scs_create(m_getter))
#define ADD_PROPERTYINO(m_property, m_setter, m_getter, m_index) ClassDB::add_property(get_class_static(), (m_property).added_usage(PROPERTY_USAGE_STORE_IF_NONONE), _scs_create(m_setter), _scs_create(m_getter), m_index)
#define ADD_GROUP(m_name, m_prefix) ClassDB::add_property_group(get_class_static(), m_name, m_prefix)

struct PropertyInfo {
Expand Down
4 changes: 2 additions & 2 deletions core/resource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,9 @@ void Resource::_bind_methods() {
ClassDB::bind_method(D_METHOD("duplicate", "subresources"), &Resource::duplicate, DEFVAL(false));
ADD_SIGNAL(MethodInfo("changed"));
ADD_GROUP("Resource", "resource_");
ADD_PROPERTYNZ(PropertyInfo(Variant::BOOL, "resource_local_to_scene"), "set_local_to_scene", "is_local_to_scene");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "resource_local_to_scene"), "set_local_to_scene", "is_local_to_scene");
ADD_PROPERTY(PropertyInfo(Variant::STRING, "resource_path", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_EDITOR), "set_path", "get_path");
ADD_PROPERTYNZ(PropertyInfo(Variant::STRING, "resource_name"), "set_name", "get_name");
ADD_PROPERTY(PropertyInfo(Variant::STRING, "resource_name"), "set_name", "get_name");

BIND_VMETHOD(MethodInfo("_setup_local_to_scene"));
}
Expand Down
112 changes: 29 additions & 83 deletions editor/editor_inspector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,50 +36,6 @@
#include "multi_node_edit.h"
#include "scene/resources/packed_scene.h"

EditorDefaultClassValueCache *EditorDefaultClassValueCache::singleton = NULL;

EditorDefaultClassValueCache *EditorDefaultClassValueCache::get_singleton() {
return singleton;
}

Variant EditorDefaultClassValueCache::get_default_value(const StringName &p_class, const StringName &p_property) {

if (!default_values.has(p_class)) {

default_values[p_class] = Map<StringName, Variant>();

if (ClassDB::can_instance(p_class)) {

Object *c = ClassDB::instance(p_class);
List<PropertyInfo> plist;
c->get_property_list(&plist);
for (List<PropertyInfo>::Element *E = plist.front(); E; E = E->next()) {
if (E->get().usage & PROPERTY_USAGE_EDITOR) {

Variant v = c->get(E->get().name);
default_values[p_class][E->get().name] = v;
}
}
memdelete(c);
}
}

if (!default_values.has(p_class)) {
return Variant();
}

if (!default_values[p_class].has(p_property)) {
return Variant();
}

return default_values[p_class][p_property];
}

EditorDefaultClassValueCache::EditorDefaultClassValueCache() {
ERR_FAIL_COND(singleton != NULL);
singleton = this;
}

Size2 EditorProperty::get_minimum_size() const {

Size2 ms;
Expand Down Expand Up @@ -418,14 +374,24 @@ bool EditorProperty::_get_instanced_node_original_property(const StringName &p_p
node = node->get_owner();
}

if (!found) {
//if not found, try default class value
Variant attempt = ClassDB::class_get_default_property_value(object->get_class_name(), property);
if (attempt.get_type() != Variant::NIL) {
found = true;
value = attempt;
}
}

return found;
}

bool EditorProperty::_is_property_different(const Variant &p_current, const Variant &p_orig, int p_usage) {
bool EditorProperty::_is_property_different(const Variant &p_current, const Variant &p_orig) {

// this is a pretty difficult function, because a property may not be saved but may have
// the flag to not save if one or if zero

//make sure there is an actual state
{
Node *node = Object::cast_to<Node>(object);
if (!node)
Expand Down Expand Up @@ -459,15 +425,6 @@ bool EditorProperty::_is_property_different(const Variant &p_current, const Vari
return false; //pointless to check if we are not comparing against anything.
}

if (p_orig.get_type() == Variant::NIL) {
// not found (was not saved)
// check if it was not saved due to being zero or one
if (p_current.is_zero() && property_usage & PROPERTY_USAGE_STORE_IF_NONZERO)
return false;
if (p_current.is_one() && property_usage & PROPERTY_USAGE_STORE_IF_NONONE)
return false;
}

if (p_current.get_type() == Variant::REAL && p_orig.get_type() == Variant::REAL) {
float a = p_current;
float b = p_orig;
Expand All @@ -478,39 +435,30 @@ bool EditorProperty::_is_property_different(const Variant &p_current, const Vari
return bool(Variant::evaluate(Variant::OP_NOT_EQUAL, p_current, p_orig));
}

bool EditorProperty::_is_instanced_node_with_original_property_different() {

bool mbi = _might_be_in_instance();
if (mbi) {
Variant vorig;
int usage = property_usage & (PROPERTY_USAGE_STORE_IF_NONONE | PROPERTY_USAGE_STORE_IF_NONZERO);
if (_get_instanced_node_original_property(property, vorig) || usage) {
Variant v = object->get(property);

if (_is_property_different(v, vorig, usage)) {
return true;
}
}
}
return false;
}

void EditorProperty::update_reload_status() {

if (property == StringName())
return; //no property, so nothing to do

bool has_reload = false;

if (EditorDefaultClassValueCache::get_singleton()) {
Variant default_value = EditorDefaultClassValueCache::get_singleton()->get_default_value(object->get_class_name(), property);
if (_might_be_in_instance()) {
//check for difference including instantiation
Variant vorig;
if (_get_instanced_node_original_property(property, vorig)) {
Variant v = object->get(property);

if (_is_property_different(v, vorig)) {
has_reload = true;
}
}
} else {
//check for difference against default class value instead
Variant default_value = ClassDB::class_get_default_property_value(object->get_class_name(), property);
if (default_value != Variant() && default_value != object->get(property)) {
has_reload = true;
}
}
if (_is_instanced_node_with_original_property_different()) {
has_reload = true;
}

if (object->call("property_can_revert", property).operator bool()) {

Expand Down Expand Up @@ -714,13 +662,11 @@ void EditorProperty::_gui_input(const Ref<InputEvent> &p_event) {
}
}

if (EditorDefaultClassValueCache::get_singleton()) {
Variant default_value = EditorDefaultClassValueCache::get_singleton()->get_default_value(object->get_class_name(), property);
if (default_value != Variant()) {
emit_signal("property_changed", property, default_value);
update_property();
return;
}
Variant default_value = ClassDB::class_get_default_property_value(object->get_class_name(), property);
if (default_value != Variant()) {
emit_signal("property_changed", property, default_value);
update_property();
return;
}
}
if (check_rect.has_point(mb->get_position())) {
Expand Down
17 changes: 1 addition & 16 deletions editor/editor_inspector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,6 @@

class UndoRedo;

class EditorDefaultClassValueCache : public Object {
GDCLASS(EditorDefaultClassValueCache, Object)

Map<StringName, Map<StringName, Variant> > default_values;

static EditorDefaultClassValueCache *singleton;

public:
static EditorDefaultClassValueCache *get_singleton();

Variant get_default_value(const StringName &p_class, const StringName &p_property);
EditorDefaultClassValueCache();
};

class EditorProperty : public Container {

GDCLASS(EditorProperty, Container)
Expand Down Expand Up @@ -85,8 +71,7 @@ class EditorProperty : public Container {
bool draw_top_bg;

bool _might_be_in_instance();
bool _is_property_different(const Variant &p_current, const Variant &p_orig, int p_usage);
bool _is_instanced_node_with_original_property_different();
bool _is_property_different(const Variant &p_current, const Variant &p_orig);
bool _get_instanced_node_original_property(const StringName &p_prop, Variant &value);
void _focusable_focused(int p_index);

Expand Down
4 changes: 1 addition & 3 deletions editor/editor_node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4767,8 +4767,6 @@ EditorNode::EditorNode() {
ResourceLoader::set_timestamp_on_load(true);
ResourceSaver::set_timestamp_on_save(true);

default_value_cache = memnew(EditorDefaultClassValueCache);

{ //register importers at the beginning, so dialogs are created with the right extensions
Ref<ResourceImporterTexture> import_texture;
import_texture.instance();
Expand Down Expand Up @@ -5897,7 +5895,7 @@ EditorNode::~EditorNode() {
memdelete(editor_plugins_force_input_forwarding);
memdelete(file_server);
memdelete(progress_hb);
memdelete(default_value_cache);

EditorSettings::destroy();
}

Expand Down
1 change: 0 additions & 1 deletion editor/editor_node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ class EditorNode : public Node {

Ref<Theme> theme;

EditorDefaultClassValueCache *default_value_cache;
PopupMenu *recent_scenes;
SceneTreeDock *scene_tree_dock;
InspectorDock *inspector_dock;
Expand Down
Loading

4 comments on commit f2e5405

@akien-mga
Copy link
Member

Choose a reason for hiding this comment

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

🎉

@endragor
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't include object default properties though, does it? We also shouldn't store values that are equal to script's default values or instanced scene's parent values.

@neikeq
Copy link
Contributor

@neikeq neikeq commented on f2e5405 Nov 10, 2018

Choose a reason for hiding this comment

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

I commented this above, but it may have been missed. If these constants are deprecated, shouldn't we keep them until 4.0? Aren't we breaking compatibility by removing them now?

@akien-mga
Copy link
Member

Choose a reason for hiding this comment

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

We have a relatively loose interpretation of "compatibility breaking" between minor versions. If you check the diff between 3.0's classref and the current master branch, you'll see that there are quite a few API removals which would be considered API breakage. But in most cases the removed APIs were outright broken, so their removal should likely not impact users.

So basically until now, if it's not going to break actual projects, it's ok-ish to remove without waiting for 4.0. It's a tradeoff between doing necessary changes and providing a stable API to users.

I'm not saying we shouldn't change our policy though, just stating how it worked until now. I'm OK with having this property readded if we fear that some plugins or tool scripts might break.

Please sign in to comment.