-
-
Notifications
You must be signed in to change notification settings - Fork 463
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
Dictionary class is missing key methods #1337
Comments
These are not exposed, and This would require either declaring How is iterating over the keys and values slower? How have you confirmed that this would be slower than using the non-exposed methods? Given the implementation in c++ the int index = 0;
for (const KeyValue<Variant, Variant> &E : _p->variant_map) {
if (index == p_index) {
return E.key;
}
index++;
}
return Variant(); It iterates over the map, which is a hash map, so the time for each operation is O(n), so nominally using this to iterate over the dictionary is O(n^2), copying the keys and values and iterating over them instead is most likely faster, quite a bit faster in fact Instead doing fetching just the keys, and then iterating over those, is nominally an O(n) operation, fetching the keys is O(n), and looking up each key in the dictionary is amortized constant |
I assumed it's slow. You are right, for iterations it should be faster to use |
Well I'd suggest confirming it's slow before opening an issue 🙂, assumptions are dangerous when they inform decisions like these You can still get the key and index as a specific index though, with those methods getting the keys and values, and they aren't meaningfully slower when doing them once (just "I want this specific entry", then just do I'd say a more useful improvement would be to add enumeration, i.e. |
Enumerations would be great! 👍 |
Do try the following for enumeration, from the example code: Variant test_variant_iterator(const Variant &p_input) {
Array output;
Variant iter;
bool is_init_valid = true;
if (!p_input.iter_init(iter, is_init_valid)) {
if (!is_init_valid) {
return "iter_init: not valid";
}
return output;
}
bool is_iter_next_valid = true;
bool is_iter_get_valid = true;
do {
if (!is_iter_next_valid) {
return "iter_next: not valid";
}
Variant value = p_input.iter_get(iter, is_iter_get_valid);
if (!is_iter_get_valid) {
return "iter_get: not valid";
}
output.push_back(((int)value) + 5);
} while (p_input.iter_next(iter, is_iter_next_valid));
if (!is_iter_next_valid) {
return "iter_next: not valid";
}
return output;
} This could be made more convenient, but at the moment I don't think there's an extension detail to indicate that a type is iterable, so would need that I think Edit: Will do some testing later to more easily do this as a #define GODOT_FOR_EACH(m_variable, m_iterable) Which then would be used so: GODOT_FOR_EACH(foo, iterable) {
bar(foo);
} |
Ideally, it would be nice to have a common iteration solution for Dictionary in both the engine module code and GDExtension code. Less #ifdefs. I'll stick to keys()/values() from now on, but |
Being able to iterate over However, since we try to match the API of the engine itself, we'd need to first add it to |
Godot version
4.2.1-stable
godot-cpp version
4.2
System information
Manjaro GNOME, NVIDIA X11, Forward+
Issue description
Dictionary is missing
get_value_at_index
,get_key_at_index
andget_valid
.get_valid
is easy to work around, but the other two are problematic. The workaround if to get keys() and values() to iterate over, and it's slow.Steps to reproduce
N/A
Minimal reproduction project
N/A
The text was updated successfully, but these errors were encountered: