godot icon indicating copy to clipboard operation
godot copied to clipboard

Replace setting by reference with ties

Open tygyh opened this issue 1 year ago • 2 comments

Shortens the method signature and make the code smaller. Pure positives!

tygyh avatar Dec 15 '24 18:12 tygyh

https://docs.godotengine.org/en/stable/contributing/development/cpp_usage_guidelines.html#auto-keyword

arkology avatar Dec 15 '24 19:12 arkology

@arkology You probably wanted to refer to the "Standard Template Library" headline, not the one regarding the "auto" keyword? ^^

But yeah, Godot codebase does not accept std::tuple, so this change will likely not be accepted.

RedMser avatar Dec 15 '24 23:12 RedMser

@RedMser previous the author also used auto keyword) I believe that if the author wanted to contribute something useful with their PR, they would have read the entire page after being pointed out one of its headlines. It's okay not to know all the rules (I don't 🙃), but a link to them was provided.

arkology avatar Dec 16 '24 07:12 arkology

if the author wanted to contribute something useful with their PR, they would have read the entire page

I don't see any mentions of tuples in the rules. I can update the docs to include an explanation of why they aren't allowed if you explain the reason to me.

tygyh avatar Dec 16 '24 07:12 tygyh

I'm really sorry if it sounds offensive. You are right, the docs don't mention it directly. Feel free to open an issue on the Godot Docs repo.

arkology avatar Dec 16 '24 09:12 arkology

I'm really sorry if it sounds offensive.

No offense taken. You were explaining the spirit of the rules and it simply didn't match the letter of the rules. Easy fix!

tygyh avatar Dec 16 '24 15:12 tygyh

I've made an issue mentioning this. Since this change isn't allowed I'll close this PR.

tygyh avatar Dec 16 '24 15:12 tygyh