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

Convert all references and instances of PoolVector to Vector #36311

Merged
merged 1 commit into from
Feb 18, 2020

Conversation

reduz
Copy link
Member

@reduz reduz commented Feb 17, 2020

Makes no sense to keep PoolVector in a world of 64 bits computers.

@fire
Copy link
Member

fire commented Feb 17, 2020

Since someone asked. Here's the design proposal for converting PoolVector to Vector.

https://docs.google.com/document/d/1HtFfENFdnmzB1XhviIqVoeGQbIB1x30vAIFtjTFW18o/edit

@aaronfranke
Copy link
Member

aaronfranke commented Feb 18, 2020

I assume PoolWhateverArray would need to be renamed to PackedWhateverArray in user code and projects, but does it behave the same, or is the behavior different?

Is there any reason to keep using Vector<String> internally when we could just use the PackedStringArray semantic, etc? It seems that right now they are used interchangeably. If the concern is with too many things depending on variant.h since it includes the typdefs, we could potentially move the typedefs to a different header file (perhaps just vector.h, in particular PackedByteArray/PackedIntArray/PackedRealArray don't need anything other than vector.h and math_defs.h).

Typed `PoolTypeArray` types are now renamed `PackedTypeArray` and are
sugar for `Vector<Type>`.
@akien-mga akien-mga force-pushed the poolvector-deprecation branch from fa32513 to 3205a92 Compare February 18, 2020 09:12
@akien-mga
Copy link
Member

I rebased to squash the commits and amended to fix build for Mono, Windows and macOS.

There are still a few fixes needed for Android, iOS and JavaScript, but since they can't build right now due to the lack of rendering backend, I prefer not to do the changes blindly.

@reduz
Copy link
Member Author

reduz commented Feb 18, 2020

@aaronfranke no the names are only in the context of Variant and used for exposing API and serialziing. They are called packed because regular arays ini Variant are just arrays of variants.

Some contributors used them in their code for unrelated stuff but its not really required.

@akien-mga akien-mga merged commit ef58910 into godotengine:master Feb 18, 2020
@akien-mga
Copy link
Member

Thanks!

@@ -646,11 +646,6 @@ uint64_t _OS::get_static_memory_peak_usage() const {
return OS::get_singleton()->get_static_memory_peak_usage();
}

uint64_t _OS::get_dynamic_memory_usage() const {
Copy link
Contributor

Choose a reason for hiding this comment

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

was this removed by mistake?

Copy link
Member

@akien-mga akien-mga Feb 18, 2020

Choose a reason for hiding this comment

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

@agar3s
Copy link

agar3s commented Feb 20, 2020

Should be this section in the docs updated?

For dynamic memory, the PoolVector<> template is provided. PoolVector is a standard vector class, and is very similar to vector in the C++ standard library. To create a PoolVector buffer, use this:

https://docs.godotengine.org/en/latest/development/cpp/core_types.html#allocating-memory

@clayjohn
Copy link
Member

@agar3s Yes. Please make an issue in the docs repo so other contributors can keep track. Or make a PR yourself if you are up to it. :)

@agar3s
Copy link

agar3s commented Feb 20, 2020

@clayjohn there is an issue already: godotengine/godot-docs#3192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants