Simplify ScriptServer::get_global_class_list#108577
Simplify ScriptServer::get_global_class_list#108577Repiteo merged 1 commit intogodotengine:masterfrom
ScriptServer::get_global_class_list#108577Conversation
editor/gui/create_dialog.cpp
Outdated
| List<StringName> complete_type_list; | ||
| ClassDB::get_class_list(&complete_type_list); | ||
| ScriptServer::get_global_class_list(&complete_type_list); | ||
| for (const StringName &type : ScriptServer::get_global_class_list()) { | ||
| complete_type_list.push_back(type); | ||
| } |
There was a problem hiding this comment.
Considering this use-case, I'd probably keep using a parameter (instead of returning a new list), but make it use LocalVector for both get_class_list and get_global_class_list. You can use SortArray to sort just the subsection of the newly added elements to preserve behavior.
There was a problem hiding this comment.
Change as suggested. I'm not sure if I should change get_class_list to use SortArray, it currently sort all values in the list (although I think it is always given an empty list). Not sure which meaning is correct.
There was a problem hiding this comment.
I think it's fine, especially since the other one only sorts the added elements. Fingers crossed nothing relies on the previous behavior.
There was a problem hiding this comment.
Add a comment to clarify the current behavior of these functions.
YYF233333
force-pushed
the
global_class_list
branch
from
fb3b269 to
2b5b9e9
Compare
editor/doc/doc_tools.cpp
Outdated
| ClassDB::get_class_list(classes); | ||
| // Move ProjectSettings, so that other classes can register properties there. | ||
| classes.move_to_back(classes.find("ProjectSettings")); | ||
| SWAP(classes[classes.find("ProjectSettings")], classes[classes.size() - 1]); |
There was a problem hiding this comment.
I'm not sure why it has to be moved to back, but if it has an effect on which other classes can register properties on it, we probably shouldn't move the final element into its place. Instead, the element should be removed and then re-added to back (same as move_to_back).
There was a problem hiding this comment.
Sorry, my bad. I must have misunderstood how move_to_back is implemented. Fixed.
editor/gui/create_dialog.cpp
Outdated
| List<StringName> complete_type_list; | ||
| ClassDB::get_class_list(&complete_type_list); | ||
| ScriptServer::get_global_class_list(&complete_type_list); | ||
| for (const StringName &type : ScriptServer::get_global_class_list()) { | ||
| complete_type_list.push_back(type); | ||
| } |
There was a problem hiding this comment.
I think it's fine, especially since the other one only sorts the added elements. Fingers crossed nothing relies on the previous behavior.
YYF233333
force-pushed
the
global_class_list
branch
3 times, most recently
from
0ee3ce3 to
5beacd6
Compare
YYF233333
force-pushed
the
global_class_list
branch
from
5beacd6 to
a50fc5a
Compare
|
Thanks! |
4 participants
The implementation of this function is rather awkward: it first copies the elements into a
List, then copies them again intor_global_classes. We can simply return the intermediateListso that users can use it directly.Since I was already working on this code, I also took the opportunity to tidy up some variable names and change unnecessary
Lists toLocalVectors.