Skip to content

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
@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