Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing OP_POWER operator to Variant #1349

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Jan 6, 2024

I don't think this breaks compatibility as there's no binary compatibility to maintain here, the mapping to the engine is done via the interface macros, but need verification here

Edit: This likely fixes Variant::evaluate which tries to map Operator to GDExtensionVariantOperator which doesn't line up

@AThousandShips AThousandShips added the bug This has been identified as a bug label Jan 6, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Jan 6, 2024
@AThousandShips AThousandShips requested a review from a team as a code owner January 6, 2024 20:04
@Naros
Copy link
Contributor

Naros commented Jan 6, 2024

Hi @AThousandShips I can confirm this will be okay in its current form and it does fix Varian::evaluate. I had to build a bridge enum locally to deal with this so it's nice to see this fixed so I can remove the bridge handler going forward.

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!

Assuming I'm understanding this right, currently Variant::evaluate() is broken with all operators after OP_POWER because missing that item means the enum in godot-cpp doesn't line up with the enum in the GDExtension interface.

In any case, this change looks great to me :-)

@akien-mga akien-mga merged commit edf1637 into godotengine:master Jan 11, 2024
12 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

@AThousandShips AThousandShips deleted the op_fix branch January 11, 2024 12:29
@limbonaut
Copy link

Yep, thanks!

@dsnopek
Copy link
Collaborator

dsnopek commented Jan 22, 2024

Cherry-picking to 4.2 in #1372

@dsnopek
Copy link
Collaborator

dsnopek commented Jan 22, 2024

Cherry-picking to 4.1 in #1373

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.

Variant::Operation::OP_POWER is missing
5 participants