godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.69k stars 527 forks source link

Enforce `p_` prefixes for arguments in binds #1490

Closed AThousandShips closed 3 months ago

AThousandShips commented 3 months ago

Improves general readability of generated code and prevents potential collisions

AThousandShips commented 3 months ago

My bad, missed this:

Just making this an improvement

dsnopek commented 3 months ago

Thanks!

This makes sense to me: we use the p_ prefix in all the hand-written code, so let's do it for all the generated code too.

The changes here look good in a quick skimming! This just needs a rebase after having merged PR https://github.com/godotengine/godot-cpp/pull/1485

AThousandShips commented 3 months ago

Some of the cases like Array has some non-prefixed cases, should I fix all the generated code with this? Like String etc. as well?

Edit: Since these are actually generated with these arguments in the source with this change I'll fix it all

dsnopek commented 3 months ago

Some of the cases like Array has some non-prefixed cases, should I fix all the generated code with this? Like String etc. as well?

I think we ultimately want to change all cases, so if there aren't any roadblocks to doing it now, let's do it!

AThousandShips commented 3 months ago

Realized a few binds include r_ so will make those not include the p_, will push

dsnopek commented 2 months ago

Cherry-picked for 4.2 in PR https://github.com/godotengine/godot-cpp/pull/1527

dsnopek commented 2 months ago

Cherry-picked for 4.1 in PR https://github.com/godotengine/godot-cpp/pull/1529