godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #91760 from groud/fix_PropertyListHelper_not_handling_array_size 

Fix `PropertyListHelper::_get_property` returning a valid value even if an index is outside the array valid indices
This commit is contained in:
Rémi Verschelde 2024-05-10 10:48:42 +02:00
parent da5714ad6e 7705265477
commit 9c388ce5d3
No known key found for this signature in database
GPG Key ID: C3336907360768E1
12 changed files with 52 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
editor/gui/editor_file_dialog.cpp
View File

@ -1964,6 +1964,7 @@ void EditorFileDialog::_bind_methods() {
Option defaults;
base_property_helper.set_prefix("option_");
base_property_helper.set_array_length_getter(&EditorFileDialog::get_option_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "name"), defaults.name, &EditorFileDialog::set_option_name, &EditorFileDialog::get_option_name);
base_property_helper.register_property(PropertyInfo(Variant::PACKED_STRING_ARRAY, "values"), defaults.values, &EditorFileDialog::set_option_values, &EditorFileDialog::get_option_values);
base_property_helper.register_property(PropertyInfo(Variant::INT, "default"), defaults.default_idx, &EditorFileDialog::set_option_default, &EditorFileDialog::get_option_default);

29
editor/scene_tree_dock.cpp
View File

@ -2865,21 +2865,24 @@ void SceneTreeDock::replace_node(Node *p_node, Node *p_by_node) {
void SceneTreeDock::_replace_node(Node *p_node, Node *p_by_node, bool p_keep_properties, bool p_remove_old) {
ERR_FAIL_COND_MSG(!p_node->is_inside_tree(), "_replace_node() can't be called on a node outside of tree. You might have called it twice.");
Node *n = p_node;
Node *oldnode = p_node;
Node *newnode = p_by_node;
if (p_keep_properties) {
Node *default_oldnode = Object::cast_to<Node>(ClassDB::instantiate(n->get_class()));
Node *default_oldnode = Object::cast_to<Node>(ClassDB::instantiate(oldnode->get_class()));
List<PropertyInfo> pinfo;
n->get_property_list(&pinfo);
oldnode->get_property_list(&pinfo);
for (const PropertyInfo &E : pinfo) {
if (!(E.usage & PROPERTY_USAGE_STORAGE)) {
continue;
}
if (default_oldnode->get(E.name) != n->get(E.name)) {
newnode->set(E.name, n->get(E.name));
bool valid;
const Variant &default_val = default_oldnode->get(E.name, &valid);
if (!valid || default_val != oldnode->get(E.name)) {
newnode->set(E.name, oldnode->get(E.name));
}
}
@ -2891,10 +2894,10 @@ void SceneTreeDock::_replace_node(Node *p_node, Node *p_by_node, bool p_keep_pro
//reconnect signals
List<MethodInfo> sl;
n->get_signal_list(&sl);
oldnode->get_signal_list(&sl);
for (const MethodInfo &E : sl) {
List<Object::Connection> cl;
n->get_signal_connection_list(E.name, &cl);
oldnode->get_signal_connection_list(E.name, &cl);
for (const Object::Connection &c : cl) {
if (!(c.flags & Object::CONNECT_PERSIST)) {
@ -2904,15 +2907,15 @@ void SceneTreeDock::_replace_node(Node *p_node, Node *p_by_node, bool p_keep_pro
}
}
String newname = n->get_name();
String newname = oldnode->get_name();
List<Node *> to_erase;
for (int i = 0; i < n->get_child_count(); i++) {
if (n->get_child(i)->get_owner() == nullptr && n->is_owned_by_parent()) {
to_erase.push_back(n->get_child(i));
for (int i = 0; i < oldnode->get_child_count(); i++) {
if (oldnode->get_child(i)->get_owner() == nullptr && oldnode->is_owned_by_parent()) {
to_erase.push_back(oldnode->get_child(i));
}
}
n->replace_by(newnode, true);
oldnode->replace_by(newnode, true);
//small hack to make collisionshapes and other kind of nodes to work
for (int i = 0; i < newnode->get_child_count(); i++) {
@ -2928,7 +2931,7 @@ void SceneTreeDock::_replace_node(Node *p_node, Node *p_by_node, bool p_keep_pro
_push_item(newnode);
if (p_remove_old) {
memdelete(n);
memdelete(oldnode);
while (to_erase.front()) {
memdelete(to_erase.front()->get());

1
scene/2d/tile_map.cpp
View File

@ -1073,6 +1073,7 @@ TileMap::TileMap() {
TileMapLayer *defaults = memnew(TileMapLayer);
base_property_helper.set_prefix("layer_");
base_property_helper.set_array_length_getter(&TileMap::get_layers_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "name"), defaults->get_name(), &TileMap::set_layer_name, &TileMap::get_layer_name);
base_property_helper.register_property(PropertyInfo(Variant::BOOL, "enabled"), defaults->is_enabled(), &TileMap::set_layer_enabled, &TileMap::is_layer_enabled);
base_property_helper.register_property(PropertyInfo(Variant::COLOR, "modulate"), defaults->get_modulate(), &TileMap::set_layer_modulate, &TileMap::get_layer_modulate);

1
scene/gui/file_dialog.cpp
View File

@ -1344,6 +1344,7 @@ void FileDialog::_bind_methods() {
Option defaults;
base_property_helper.set_prefix("option_");
base_property_helper.set_array_length_getter(&FileDialog::get_option_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "name"), defaults.name, &FileDialog::set_option_name, &FileDialog::get_option_name);
base_property_helper.register_property(PropertyInfo(Variant::PACKED_STRING_ARRAY, "values"), defaults.values, &FileDialog::set_option_values, &FileDialog::get_option_values);
base_property_helper.register_property(PropertyInfo(Variant::INT, "default"), defaults.default_idx, &FileDialog::set_option_default, &FileDialog::get_option_default);

1
scene/gui/item_list.cpp
View File

@ -1882,6 +1882,7 @@ void ItemList::_bind_methods() {
Item defaults(true);
base_property_helper.set_prefix("item_");
base_property_helper.set_array_length_getter(&ItemList::get_item_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text, &ItemList::set_item_text, &ItemList::get_item_text);
base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, &ItemList::set_item_icon, &ItemList::get_item_icon);
base_property_helper.register_property(PropertyInfo(Variant::BOOL, "selectable"), defaults.selectable, &ItemList::set_item_selectable, &ItemList::is_item_selectable);

1
scene/gui/menu_button.cpp
View File

@ -190,6 +190,7 @@ void MenuButton::_bind_methods() {
PopupMenu::Item defaults(true);
base_property_helper.set_prefix("popup/item_");
base_property_helper.set_array_length_getter(&MenuButton::get_item_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text);
base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon);
base_property_helper.register_property(PropertyInfo(Variant::INT, "checkable", PROPERTY_HINT_ENUM, "No,As Checkbox,As Radio Button"), defaults.checkable_type);

1
scene/gui/option_button.cpp
View File

@ -571,6 +571,7 @@ void OptionButton::_bind_methods() {
PopupMenu::Item defaults(true);
base_property_helper.set_prefix("popup/item_");
base_property_helper.set_array_length_getter(&OptionButton::get_item_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text, &OptionButton::_dummy_setter, &OptionButton::get_item_text);
base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, &OptionButton::_dummy_setter, &OptionButton::get_item_icon);
base_property_helper.register_property(PropertyInfo(Variant::INT, "id", PROPERTY_HINT_RANGE, "0,10,1,or_greater"), defaults.id, &OptionButton::_dummy_setter, &OptionButton::get_item_id);

1
scene/gui/popup_menu.cpp
View File

@ -2808,6 +2808,7 @@ void PopupMenu::_bind_methods() {
Item defaults(true);
base_property_helper.set_prefix("item_");
base_property_helper.set_array_length_getter(&PopupMenu::get_item_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text, &PopupMenu::set_item_text, &PopupMenu::get_item_text);
base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, &PopupMenu::set_item_icon, &PopupMenu::get_item_icon);
base_property_helper.register_property(PropertyInfo(Variant::INT, "checkable", PROPERTY_HINT_ENUM, "No,As checkbox,As radio button"), defaults.checkable_type, &PopupMenu::_set_item_checkable_type, &PopupMenu::_get_item_checkable_type);

1
scene/gui/tab_bar.cpp
View File

@ -1868,6 +1868,7 @@ void TabBar::_bind_methods() {
Tab defaults(true);
base_property_helper.set_prefix("tab_");
base_property_helper.set_array_length_getter(&TabBar::get_tab_count);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "title"), defaults.text, &TabBar::set_tab_title, &TabBar::get_tab_title);
base_property_helper.register_property(PropertyInfo(Variant::STRING, "tooltip"), defaults.tooltip, &TabBar::set_tab_tooltip, &TabBar::get_tab_tooltip);
base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, &TabBar::set_tab_icon, &TabBar::get_tab_icon);

26
scene/property_list_helper.cpp
View File

@ -36,14 +36,17 @@ const PropertyListHelper::Property *PropertyListHelper::_get_property(const Stri
return nullptr;
}
{
const String index_string = components[0].trim_prefix(prefix);
if (!index_string.is_valid_int()) {
return nullptr;
}
*r_index = index_string.to_int();
const String index_string = components[0].trim_prefix(prefix);
if (!index_string.is_valid_int()) {
return nullptr;
}
int index = index_string.to_int();
if (index < 0 || index >= _call_array_length_getter()) {
return nullptr;
}
*r_index = index;
return property_list.getptr(components[1]);
}
@ -66,6 +69,11 @@ Variant PropertyListHelper::_call_getter(const Property *p_property, int p_index
return p_property->getter->call(object, argptrs, 1, ce);
}
int PropertyListHelper::_call_array_length_getter() const {
Callable::CallError ce;
return array_length_getter->call(object, nullptr, 0, ce);
}
void PropertyListHelper::set_prefix(const String &p_prefix) {
prefix = p_prefix;
}
@ -83,7 +91,13 @@ bool PropertyListHelper::is_initialized() const {
}
void PropertyListHelper::setup_for_instance(const PropertyListHelper &p_base, Object *p_object) {
DEV_ASSERT(!p_base.prefix.is_empty());
DEV_ASSERT(p_base.array_length_getter != nullptr);
DEV_ASSERT(!p_base.property_list.is_empty());
DEV_ASSERT(p_object != nullptr);
prefix = p_base.prefix;
array_length_getter = p_base.array_length_getter;
property_list = p_base.property_list;
object = p_object;
}

7
scene/property_list_helper.h
View File

@ -43,15 +43,22 @@ class PropertyListHelper {
};
String prefix;
MethodBind *array_length_getter = nullptr;
HashMap<String, Property> property_list;
Object *object = nullptr;
const Property *_get_property(const String &p_property, int *r_index) const;
void _call_setter(const MethodBind *p_setter, int p_index, const Variant &p_value) const;
Variant _call_getter(const Property *p_property, int p_index) const;
int _call_array_length_getter() const;
public:
void set_prefix(const String &p_prefix);
template <typename G>
void set_array_length_getter(G p_array_length_getter) {
array_length_getter = create_method_bind(p_array_length_getter);
}
// Register property without setter/getter. Only use when you don't need PropertyListHelper for _set/_get logic.
void register_property(const PropertyInfo &p_info, const Variant &p_default);

1
servers/audio/audio_stream.cpp
View File

@ -711,6 +711,7 @@ void AudioStreamRandomizer::_bind_methods() {
PoolEntry defaults;
base_property_helper.set_prefix("stream_");
base_property_helper.set_array_length_getter(&AudioStreamRandomizer::get_streams_count);
base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "stream", PROPERTY_HINT_RESOURCE_TYPE, "AudioStream"), defaults.stream, &AudioStreamRandomizer::set_stream, &AudioStreamRandomizer::get_stream);
base_property_helper.register_property(PropertyInfo(Variant::FLOAT, "weight", PROPERTY_HINT_RANGE, "0,100,0.001,or_greater"), defaults.weight, &AudioStreamRandomizer::set_stream_probability_weight, &AudioStreamRandomizer::get_stream_probability_weight);
}