Skip to content

Conversation

@mihe
Copy link
Contributor

@mihe mihe commented Aug 21, 2024

Fixes #1556.

This fixes the generation of operator!, operator&& and operator||, as they were not being mapped correctly from their extension_api.json name of not, and and or respectively, resulting in them showing up as operatornot, operatorand and operatoror respectively.

Note that operatorand and operatoror never actually showed up anywhere in the generated C++ headers, as they only exist on built-in types in extension_api.json currently.

I also fixed another related issue, which is that the ** operator wasn't being skipped like the in and xor operators have been until now, neither of which make no sense in C++ of course. This is now done through a new helper function called is_valid_cpp_operator.

I suppose this is technically a breaking change, so I'm not sure if there's perhaps some argument for leaving the old operatornot methods intact.

@mihe mihe requested a review from a team as a code owner August 21, 2024 18:35
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Good catch :-)

I suppose this is technically a breaking change, so I'm not sure if there's perhaps some argument for leaving the old operatornot methods intact.

Yes, however, I personally think this is fine. It's a source compatibility breakage regarding functions that it's super unlikely that anyone was calling.

@dsnopek dsnopek merged commit 265412c into godotengine:master Aug 22, 2024
@mihe mihe deleted the cpp-operators branch August 22, 2024 21:58
@dsnopek
Copy link
Collaborator

dsnopek commented Sep 3, 2024

Cherry-picked for 4.3 in PR #1569

@dsnopek
Copy link
Collaborator

dsnopek commented Sep 3, 2024

Cherry-picked for 4.2 in PR #1570

@dsnopek
Copy link
Collaborator

dsnopek commented Sep 4, 2024

Cherry-picked for 4.1 in PR #1572

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This has been identified as a bug

3 participants