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

Jump to bottom

Fix PropertyListHelper::_get_property returning a valid value even if an index is outside the array valid indices #91760

Merged
merged 1 commit into from
May 10, 2024
+52 −19
Merged

Fix PropertyListHelper::_get_property returning a valid value even if an index is outside the array valid indices #91760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions editor/gui/editor_file_dialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 16 additions & 13 deletions editor/scene_tree_dock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand All @@ -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)) {
Expand All @@ -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++) {
Expand All @@ -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());
Expand Down
1 change: 1 addition & 0 deletions scene/2d/tile_map.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions scene/gui/file_dialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions scene/gui/item_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions scene/gui/menu_button.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions scene/gui/option_button.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions scene/gui/popup_menu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions scene/gui/tab_bar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 20 additions & 6 deletions scene/property_list_helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
7 changes: 7 additions & 0 deletions scene/property_list_helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions servers/audio/audio_stream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading