Skip to content

Fix VARIANT_ENUM_CAST to support enum class#1914

Open
daniellopez2002 wants to merge 2 commits intogodotengine:4.5from
daniellopez2002:4.5
Open

Fix VARIANT_ENUM_CAST to support enum class#1914
daniellopez2002 wants to merge 2 commits intogodotengine:4.5from
daniellopez2002:4.5

Conversation

@daniellopez2002
Copy link

@daniellopez2002 daniellopez2002 commented Jan 28, 2026

Problem

VARIANT_ENUM_CAST assumes implicit conversion from enum values to int64_t when
encoding arguments. This is invalid for enum class types according to the C++
standard and fails to compile on MSVC.

Solution

Explicitly cast enum values to int64_t when encoding, matching the behavior
already implemented in VARIANT_BITFIELD_CAST.

Notes

  • ABI-safe
  • Backward-compatible
  • Improves MSVC compatibility

Fixes #1917

@daniellopez2002 daniellopez2002 requested a review from a team as a code owner January 28, 2026 22:12
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 28, 2026

Thanks!

We still don't "officially" support enum classes, although, if this is really all it takes to make them work in godot-cpp, the change is small enough that it could be alright. I remember when discussing binding enum classes in Godot itself, it turned out there were a lot more changes than initially expected

@daniellopez2002
Copy link
Author

Can it be merged? Or does it need any changes or improvements?

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.

It looks like Godot itself already basically has this cast via this code:

https://github.com/godotengine/godot/blob/98782b6c8c9cabe0fb7c80bc62640735ecb076d3/core/variant/method_ptrcall.h#L243-L246

We should eventually sync the changes from godotengine/godot#105231 into godot-cpp, but I think this is OK for now

@dsnopek dsnopek added the bug This has been identified as a bug label Feb 1, 2026
@dsnopek dsnopek added this to the 10.x milestone Feb 1, 2026
@daniellopez2002
Copy link
Author

Thanks a lot for the clarification and for the review!

This is actually my first time contributing a PR to an open source project, so I really appreciate the feedback and the approval. Happy to help later on if syncing with core changes becomes necessary.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants