Add PackedByteArray conversion to PackedVector2Array, PackedVector3Array, PackedVector4Array and PackedColorArray

[OsakiTsukiko]

adds to_vector2_array, to_vector3_array, and to_color_array methods on PackedByteArray to convert to the corresponding types, as prior you could only convert from said type to PackedByteArray (with to_byte_array)

• Bugsquad edit, closes: PackedByteArray: add to_vector2_array and to_vector3_array functions godot-proposals#8139

[OsakiTsukiko]

I haven't done one for PackedStringArray as I think the to_byte_array method on PackedStringArray is broken (I'll look some more into it tomorrow)

[bitsawer]

Note that for Vector2/3/4 the underlaying data type is not always a float, it's real_t which can be a 32-bit float or 64-bit double depending on Godot build settings. But it might be best to just use full class size. So instead of using sizeof(float) * 2 for Vector2, just use sizeof(Vector2) directly. You'll also have to update the documentation for this.

PackedStringArray also only stores the String objects themselves, which are basically just pointers to the actual text data. It doesn't actually store tightly packed text data or anything like that. So there is probably no point in adding that converter.

Also remember to squish your commits into a single commit, see PR Workflow.

[OsakiTsukiko]

Note that for Vector2/3/4 the underlaying data type is not always a float, it's real_t which can be a 32-bit float or 64-bit double depending on Godot build settings. But it might be best to just use full class size. So instead of using sizeof(float) * 2 for Vector2, just use sizeof(Vector2) directly. You'll also have to update the documentation for this.

Oh, I wasn't sure if I could use sizeof on custom data types

PackedStringArray also only stores the String objects themselves, which are basically just pointers to the actual text data. It doesn't actually store tightly packed text data or anything like that. So there is probably no point in adding that converter.

Hmm, yea, I thought so. Is there any point to the to_byte_array method on the String? I do not see much use for a byte array of references, and it's quite unintuitive, especially for beginners (It doesn't really specify anywhere in the docs that it only stores references, but that it packs data tightly (in-built docs), and even worse on the online docs: Returns a PackedByteArray with each string encoded as bytes.)

Also remember to squish your commits into a single commit, see PR Workflow.

Oh, Sure!

[kleonc]

Duplicate/alternative: #65811.

[ghost]

why isnt this approved already? friend is running this PR on his own custom compiled version with no problem, this is essential for compute shaders, and compute shaders are a very big thing

[Calinou]

why isnt this approved already? friend is running this PR on his own custom compiled version with no problem, this is essential for compute shaders, and compute shaders are a very big thing

There is a related proposal, but it's not very focused on this particular feature. Not much consensus has been reached, and we don't know whether this PR or #65811 is better yet.

[AThousandShips]

Generally I think this looks great! I'll see if I can't find the time soon to provide some hints for the documentation as I think it needs some suggestions, but once I've given those I'll be approving this as I think it's both a good feature and looks good

[AThousandShips]

Some ideas for improving the documentation, the details on the precision is available on the respective vector type page so should be sufficient to link there

[AThousandShips]

Please squash your commits into one, see here

[OsakiTsukiko]

ok, I hope I did it right 🥲

[AThousandShips]

Looks great! Just one last detail, could you please update the title of the commit to match the PR to make it a bit clearer without low level details of the code, you do this with:

git commit --amend
git push --force

Thank you and this looks great!

[nvl-ez]

Will it be merged? Since it is essential for working with compute shaders.

[AThousandShips]

It will hopefully be merged early in 4.5, we're in feature freeze now

[tjpalmer]

It will hopefully be merged early in 4.5, we're in feature freeze now

Looking forward to this merge for 4.5!

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

[OsakiTsukiko]

Thanks! And congrats for your first merged Godot contribution 🎉

Thank you!
Tho it's not my first 😅

[akien-mga]

First with this GitHub identity / username at least according to GitHub and the git log :D

[OsakiTsukiko]

First with this GitHub identity / username at least according to GitHub and the git log :D

I thought that was weird, so I looked a bit into it, and it doesn't show in the log because it's not on the master branch. (Tho tbf that was just a small fix, so this is my first actual contribution. I was just interested as to how you knew this was my first commit).