Skip to content

Conversation

dementive
Copy link

std::list was being used in in one place for method args in class_db.hpp. This replaces it with Godot's List.

This works fine but after looking at the code I'm pretty sure this shouldn't even be using a List at all. It's only used in MethodDefinition's args member but internally Godot uses a Vector<StringName> for this but godot-cpp is using a List<StringName>. godot-cpp's D_METHOD works different than the internal Godot one though so maybe this is intentional? Either way I don't want to make that change until #1841 gets merged as they will conflict.

@dementive dementive requested a review from a team as a code owner September 10, 2025 22:14
@Calinou Calinou added the enhancement This is an enhancement on the current functionality label Sep 10, 2025
@Ivorforce
Copy link
Member

It's possible that Godot used to use List as well, but it was optimized to use Vector later.
I agree that List very likely isn't optimal here, let's take the opportunity to use LocalVector instead.

@dementive
Copy link
Author

dementive commented Sep 12, 2025

To change this to a Vector I kind of have to rework a lot of things so I don't want to do it in this PR.

I compared the MethodDefinition struct and D_METHOD function in godot-cpp to the one's in godot and they do the same thing but for some reason the way they are implemented is completely different. Differences I found are:

  • The MethodDefinition struct and D_METHOD function only exist when DEBUG_ENABLED is set in godot but in godot-cpp they always exist.
  • D_METHOD's implementation in godot-cpp is different than the one in godot so it's hard to just make it use a Vector without reworking a lot of stuff. Although the only time the List ever gets used is when it gets copied into a Vector. The reason it's different might just be because at some point it had to use stl containers though?
  • The MethodBind class in godot-cpp has a member argument_names that the List in MethodDefinition gets copied to. In godot this and it's associated MethodDefinition get/set_argument_names functions are debug only but here it isn't.

I'm not sure if this stuff is an oversight and they are meant to be debug only or if they are actually needed, it's hard to tell but it doesn't look like they are actually used so the differences are confusing. A lot of this came from and hasn't been changed since this commit 4 years ago https://github.com/godotengine/godot-cpp/tree/e4ed4897, which is the same commit that added all the stl containers so maybe there was some reason for it back then but I can't see one now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants