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 support for PackedVector4Array #1456

Merged
merged 1 commit into from
May 7, 2024

Conversation

dsnopek
Copy link
Collaborator

@dsnopek dsnopek commented May 6, 2024

Fixes #1454

Adds support for PackedVector4Array from Godot PR godotengine/godot#85474

@dsnopek dsnopek added the enhancement This is an enhancement on the current functionality label May 6, 2024
@dsnopek dsnopek added this to the 4.x milestone May 6, 2024
@dsnopek dsnopek requested a review from a team as a code owner May 6, 2024 19:11
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM

@dsnopek dsnopek merged commit f5c2b38 into godotengine:master May 7, 2024
12 checks passed
@YuriSizov
Copy link
Contributor

Should this be considered a compatibility breaking change? Without a compatible engine version extensions built with this commit included no longer work, unfortunately. This means all existing official builds of 4.3, I think.

@dsnopek
Copy link
Collaborator Author

dsnopek commented May 16, 2024

The master branch of godot-cpp is generally only compatible with the master branch of Godot. Once we start doing -beta and -rc releases for 4.3, we'll make an issue that lists commits that are compatible with those version (for example, see #1267). Historically, we haven't done anything for compatibility with -dev releases, though.

Without a compatible engine version extensions built with this commit included no longer work, unfortunately.

It's true the other way too. If you have an extension_api.json from Godot built after PackedVector4Array was added, then godot-cpp from before this PR won't build anymore. So, for godot-cpp master to continue working with Godot master, we have to include this PR. Godot master is a moving target, and godot-cpp master needs to change to keep up.

Anyway, I guess if we wanted to start worrying about compatibility for -dev releases, we could keep track of commit hashes like we do for -beta and -rc releases?

@YuriSizov
Copy link
Contributor

Godot master is a moving target, and godot-cpp master needs to change to keep up.

Yes, I understand that. But even so, in the engine repo one way changes like that are marked as breaking compat. Although I see that the label is not really used here. It would still be helpful to know when such changes occur.

Anyway, I guess if we wanted to start worrying about compatibility for -dev releases, we could keep track of commit hashes like we do for -beta and -rc releases?

My main point was having information. I have no idea if it's feasible or reasonable to worry about backwards compatibility with such changes. Right now you find out that the commit is incompatible by trying it and building it and trying to run it, which is more time consuming than checking for a label on recently merged PRs 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement PackedVector4Array
4 participants