godot
godot copied to clipboard
Add PackedByteArray conversion to PackedVector2Array, PackedVector3Array, PackedVector4Array and PackedColorArray
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: https://github.com/godotengine/godot-proposals/issues/8139
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)
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.
Note that for Vector2/3/4 the underlaying data type is not always a
float, it'sreal_twhich 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 usingsizeof(float) * 2for Vector2, just usesizeof(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!
Duplicate/alternative: #65811.
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
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 https://github.com/godotengine/godot/pull/65811 is better yet.
Please squash your commits into one, see here
ok, I hope I did it right 🥲
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!
Will it be merged? Since it is essential for working with compute shaders.
It will hopefully be merged early in 4.5, we're in feature freeze now
It will hopefully be merged early in 4.5, we're in feature freeze now
Looking forward to this merge for 4.5!
Thanks! And congrats for your first merged Godot contribution :tada:
Thanks! And congrats for your first merged Godot contribution :tada:
Thank you! Tho it's not my first 😅
First with this GitHub identity / username at least according to GitHub and the git log :D
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).