-
-
Notifications
You must be signed in to change notification settings - Fork 23.8k
Rename server free functions to free_rid to match exposed API
#107139
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
Conversation
Ivorforce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully agree, it's definitely worth the churn to make APIs consistent in- and externally.
Lately we've been a bit more mindful about API breakage. It would be possible to expose a [[deprecated]] void free function in addition the free_rid to make the transition moother for modules. What do you think?
free functions to free_rid to match exposed API
Repiteo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stylistic nitpick aside, this looks good!
95cb338 to
33a07e9
Compare
|
Needs rebase |
33a07e9 to
0071372
Compare
|
Needs another rebase 🙃 |
0071372 to
68f0902
Compare
|
@Repiteo Rebased and retested. |
|
I bet you'll never guess what I'm gonna say |
68f0902 to
9fbf580
Compare
|
Thanks! |
In the current master before this PR, the servers had these functions registered with the name
"free_rid":I was working on some C++ code that uses RenderingServer and trying to make it compatible with both in-engine module code and GDExtension C++ godot-cpp, and I found that the exposed API is different from the internal API for no reason. The name
free_ridis clearer and avoids confusion with freeingObjects. Furthermore, TextServer already uses the namefree_ridfor its internal function.This PR renames the internal
freefunction on NavigationServer*D, PhysicsServer*D, RenderingDevice, and RenderingServer tofree_ridto match the exposed API in GDScript and GDExtension and match TextServer.Third-party modules can support multiple Godot versions by using
free_ridand conditionally having#define free_rid freedepending on the Godot version in files that use the modified servers.