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

Check that GDExtension is opened by compatible Godot version #1208

Merged
merged 1 commit into from
Sep 9, 2023

Conversation

dsnopek
Copy link
Collaborator

@dsnopek dsnopek commented Aug 14, 2023

This builds on PR #1193 and adds something that we discussed on issue #1195:

That is, godot-cpp will compare the version of Godot it was built for (based on the extension_api.json that was used) with the version of Godot that is loading it, and ensure that they are compatible, ie. the version of Godot is equal to or greater than the version godot-cpp was built for.

In order to do this, it also:

  • Updates the extension_api.json in the repo for Godot 4.2-dev3, since we still had the one from 4.1.1 in there.
  • Fixes up the way that we're printing early error messages that happen before godot-cpp is fully initialized. In my testing, I was getting crashes, which I think were caused by the older method using String, because we made some small incompatible changes to String in 4.2. This highlights the need to abort early if an older Godot tries to load a newer GDExtension!

@dsnopek dsnopek added bug This has been identified as a bug enhancement This is an enhancement on the current functionality labels Aug 14, 2023
@dsnopek dsnopek requested a review from a team as a code owner August 14, 2023 21:10
@dsnopek dsnopek force-pushed the check-godot-version branch from bfc674d to dd22af9 Compare August 14, 2023 21:13
Copy link
Contributor

@paddy-exe paddy-exe left a comment

Choose a reason for hiding this comment

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

Discussed in GDExtension meeting. Looks good to be except for the point below.
I assume you want to wait for the 4.2 release to merge this? Otherwise the changes to project.godot and the added extension_api.json should be removed

@dsnopek
Copy link
Collaborator Author

dsnopek commented Sep 8, 2023

Thanks!

I assume you want to wait for the 4.2 release to merge this? Otherwise the changes to project.godot and the added extension_api.json should be removed

No, we've already merged plenty of Godot 4.2 specific changes into godot-cpp's master branch, so I think those changes should be fine. However, I guess they mean I can't do a straight cherry-pick over to the 4.1 branch. :-/ But I could either make a 4.1-specific PR or just not cherry-pick this one.

@paddy-exe
Copy link
Contributor

No, we've already merged plenty of Godot 4.2 specific changes into godot-cpp's master branch, so I think those changes should be fine. However, I guess they mean I can't do a straight cherry-pick over to the 4.1 branch. :-/ But I could either make a 4.1-specific PR or just not cherry-pick this one.

You are absolutely right. Well in that case I will leave the decision up to you👍🏻

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 enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants