Add const lvalue ref to container parameters

[Muller-Castro]

Adds const lvalue reference to the core container types specified in the documentation that are declared in function parameters.

core/*: #86966
editor/*: #88368
modules/*: #88915
platform/*: #88971
servers/*: #88972
scene/*: #88974
others: #88975

[reduz]

This would probably be good to rebase, its a nice clean up.

[YuriSizov]

As mentioned above, needs a rebase, but is a welcome change.

[AThousandShips]

Oh my bad, I think there was some communication mixup here, I meant to do this to the methods you already modified

Would you be able to split this back up? I'd suggest creating a new branch with this state, and then pulling the old version i.e. the commit 9062dee on this branch, and we can do it in two parts?

[AThousandShips]

If you need help with resetting this please don't hesitate to ask, I'd prefer to merge this with the original set of changes and review the remaining later as it's much larger in scope 🙂

[Muller-Castro]

Will it be necessary to create another PR with that old branch? Because the commit doesn't change the engine's behavior and the changes are very simple

[AThousandShips]

You'd create a new branch from this one, and then reset this one, you can then create a new PR with the larger changes

The changes needs to be reviewed to prevent any issues, it can have messy side effects that can be hard to predict, your current changes aren't trivial and involves copying data in some cases, that'd require a more thorough review

Either this can be merged now with the original changes, or reviewed later with a larger scope, I'm not comfortable merging it without further review in its current state 🙂

Seeing it's almost 500 files it'd take me a very long time to review and I at least can't dedicate that time at the moment, so even with those changes separated off this should really be at least a few different PRs to aid review

[AThousandShips]

This, for example, isn't a valid change, the argument is mutated, and should instead be:

Suggested change

	ResourceLoader::ThreadLoadStatus ResourceLoader::load_threaded_get_status(const String &p_path, const Array &r_progress) {
	ResourceLoader::ThreadLoadStatus ResourceLoader::load_threaded_get_status(const String &p_path, Array &r_progress) {

So important to get this in a separate thing for cases that aren't obvious or simple

Note that this change happens to still work due to reference counting, but it's not the correct change, and there's a few more at least just by basically looking over a few files, so that needs deeper evaluation

See above, awaiting changes

[Muller-Castro]

I duplicated the branch then rollbacked using this command: git reset --hard 9062dee and then rebased to see if there was any conflict.

Should I pull another request with the remaining changes?

[AThousandShips]

They should really be split into a few different PRs, separated by part of the engine, like core, scene, modules, etc., over 400 changes aren't easy or manageable to review

Easiest way is to create branches, then using git reset master which doesn't update the files, just the inclusion, then add the individual files to the commit, like git add core/*

[akien-mga]

Thanks for your patience and great work!