Use const ref parameters in the Web modules#110977
Merged
Repiteo merged 1 commit intogodotengine:masterfrom Sep 30, 2025
Merged
Use const ref parameters in the Web modules#110977Repiteo merged 1 commit intogodotengine:masterfrom
Repiteo merged 1 commit intogodotengine:masterfrom
Conversation
dsnopek
reviewed
Sep 28, 2025
Contributor
dsnopek
left a comment
dsnopek
left a comment
There was a problem hiding this comment.
Thanks!
I'm in support of the changes to the function arguments: those make this code consistent with the style used throughout the rest of Godot's code base. However, as I noted on the other PR, perhaps the other changes should be left out for now?
(I'm a little embarrassed that I didn't do const String & in the WebXR code :-/)
aaronfranke
force-pushed
the
const-ref-param-webrtc
branch
from
e24bcc3 to
ac2e016
Compare
Contributor
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR changes parameters in the WebRTC, WebSocket, and WebXR modules to const refs:
const Type &. This change prevents making unnecessary copies of parameters when calling functions, including when the bindings call the functions.Motivation: I tried changing the copy constructors of container types like Array to explicit, but this creates a problem for non-const-ref parameters of container types in the bindings. Right now, the bindings are implicitly making copies of this data, and the bindings do not compile anymore if this is changed to explicit. See the discussions in PR #107377 and PR #110912 for more information.
Only doing this for container types would be sufficient for that purpose, but that would be a massive change throughout the entire codebase. I think it would be better for reviewability if I open multiple PRs targeting specific parts of the codebase, so for example, this PR can be reviewed by the maintainers of the web platform. And, while I'm at it, I also made this change for types such as
StringandRef<>to fix up those too.This PR also contains a small number of other changes to parameters, such as using snake_case names consistently.