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.

Sign up for GitHub

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

Jump to bottom

Use Packed*Array over Vector<T> in Variant #67220

Merged
merged 1 commit into from
Feb 7, 2024
Merged

Use Packed*Array over Vector<T> in Variant #67220

merged 1 commit into from
Feb 7, 2024

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Oct 10, 2022 •
edited
Loading

Also shuffles some method definitions and declarations to be more consistent with the way the Variant types are ordered across the codebase.

... Also removes an unnecessary JSON assign (JSON::stringify can now be accessed statically).

@Mickeon Mickeon requested a review from a team as a code owner October 10, 2022 21:12
@Mickeon Mickeon force-pushed the variant-packing branch 2 times, most recently from d9ba952 to a97f1a4 Compare October 10, 2022 21:38
@Mickeon Mickeon mentioned this pull request Oct 10, 2022
Merged
bruvzg
bruvzg reviewed Oct 11, 2022
View reviewed changes
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

It seems fine, and more consistent with GDExtension code. Is there a practical reason for the rename, or it's just a style change?

@Mickeon
Copy link
Contributor Author

Mickeon commented Oct 11, 2022 •
edited
Loading

Mostly stylistical, but makes it much easier to tell what Vector<T> is exposed to the Engine and what isn't. Variant can sometimes contain those.
For example, Vector<Plane3> isn't available as a Variant type, but Vector<Color> is, and you may tell at a glance because one of them is callled PackedColorArray.

@Chaosus Chaosus added this to the 4.0 milestone Oct 14, 2022
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023
@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@AThousandShips AThousandShips modified the milestones: 4.2, 4.3 Oct 26, 2023
@adamscott
Copy link
Member

There's no CI?

@AThousandShips
Copy link
Member

It's too old, needs a new push

@Mickeon
Use Packed*Array over Vector<T> in Variant
3edb671 
Also shuffles some method definitions and declarations to be more consistent with the way the Variant types are ordered across the codebase.

And removes an unnecessary JSON assign (`JSON::stringify` can now be accessed statically)
@Mickeon
Copy link
Contributor Author

Mickeon commented Feb 6, 2024

There we go (I think).

Mickeon added a commit to Mickeon/godot that referenced this pull request Feb 6, 2024
@Mickeon
Use Packed*Array over Vector<T> in TypedArray
2c98783 
Same as godotengine#67220 but for `typed_array.h`
@akien-mga akien-mga merged commit b41403d into godotengine:master Feb 7, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the variant-packing branch February 7, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruvzg bruvzg bruvzg left review comments

@AThousandShips AThousandShips AThousandShips approved these changes

Assignees
No one assigned
Labels
enhancement topic:codestyle topic:core
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
7 participants
@Mickeon @adamscott @AThousandShips @akien-mga @bruvzg @Chaosus @YuriSizov