Remove references to PoolVector #10329
Conversation
PoolVector has not existed since 2020. It was removed in 3205a92a: godotengine/godot@3205a92a
tetrapod00
added
bug
area:manual
There was a problem hiding this comment.
Looks right to me but needs a second review from someone more familiar with the C++ codebase (are there any caveats to Vector<> that must be mentioned?)
The return types from map_get_path are now correct.
| locked until they go out of scope. However, PoolVectors should be used | ||
| for small, dynamic memory operations, as read() and write() are too slow for a | ||
| large amount of accesses. | ||
| For dynamic memory, use Vector<>. |
There was a problem hiding this comment.
This page should also mention LocalVector, and Packed*Vector.
Vector<>- copy-on-write and can be safely passed via public API.Packed*Vector- aliases for specificVector<*>types accessible via GDScript.LocalVector<>- non COW version, with less overhead, for the internal use.
There was a problem hiding this comment.
Keep in mind that this PR is fixing a years-old documentation bug. The page should mention these, but is mentioning these a hard enough requirement that it should block merging the bugfix PR? That is, is it acceptable for the page to say only "For dynamic memory, use Vector<>" while a separate PR is worked on, that documents the alternatives?
There was a problem hiding this comment.
Yes, it is acceptable as a bug fix. For dynamic memory, use Vector<> is correct, and in general a safe option to use, so it's OK as is.
LocalVector<> is a more advanced option with more limitations, and can be added in a different PR.
There was a problem hiding this comment.
If it's OK as a bugfix, then let's merge this one as-is.
@esainane, you're welcome to document the other options mentioned here if you want to, as another PR, after this one is merged. If you don't feel knowledgeable enough, that is also OK.
There was a problem hiding this comment.
Sure, I should be able to do something proper this Sunday.
There was a problem hiding this comment.
I think I have a suitable follow up ready to go, in case people were waiting?
There was a problem hiding this comment.
It's fine to bundle it into this PR, since it's not actually merged yet.
(I thought we might have merged this already, but without an explicit second approval from an experienced engine dev I ended up not merging it yet)
|
Thank you! I decided to merge this as a bugfix as-is. If you have a followup PR ready to document the other options, it would be very welcome 🙂 |
PoolVector has not existed since 2020. It was removed in godotengine/godot@3205a92a.
Bugsquad edit: closes #10146, possibly also #3192 and #6259