Skip to content

Array support #33

Closed
Closed
@Bromeon

Description

@Bromeon

Implement a richer API for arrays -- both Array (containing Variant) and TypedArray<T> (containing concrete types).

Inspiration can be found in gdnative::core_types::PoolArray, as well as Godot's Array documentation.

Some ideas, can also be implemented as separate pull requests:

  1. Basic methods: insertion, removal, random access, etc.
  2. Trait support: Index, IntoIterator, PartialEq, PartialOrd
    • ideally use impl_builtin_traits! for the "object relation" traits
  3. Conversions, e.g. From<Vec<T>>, also between Array and TypedArray (one way fallible)
  4. Godot special methods like shuffle(), get_typed_builtin(), ...
    • would be good to discuss this first
    • lots of Godot methods have a more idiomatic approach or name in Rust: slice(), size(), sort_custom(), ...

If possible, additions should be accompanied by unit tests. Rather less functionality but properly tested than feature-creep 🙂

Activity

chitoyuu

chitoyuu commented on Nov 26, 2022

@chitoyuu

Note on Index: the trait requires a return value of an actual &T reference, and as such a sound implementation might not be possible. It isn't in Godot 3 at least. If you're trying to write one, make doubly sure that your lifetime assumptions are correct.

Bromeon

Bromeon commented on Nov 26, 2022

@Bromeon
MemberAuthor

Very good point. If e.g. IndexMut dereference

array[i] *= 2

is not possible, we could think about a convenience function:

array.edit(i, |e| e *= 2);
chitoyuu

chitoyuu commented on Nov 27, 2022

@chitoyuu

Not sure if this makes it the "conventional" Rust name for such functions, but that signature looks pretty similar to the unstable Cell::update method: https://doc.rust-lang.org/std/cell/struct.Cell.html#method.update

I'm not exactly convinced of the "returns the new value" part of the std function though. Of course it should be possible due to Variant being reference-counted, but I wonder if it's useful to begin with. The related issue might be interesting to read.

Hanna-Kitten

Hanna-Kitten commented on Dec 17, 2022

@Hanna-Kitten

So, I am not very experienced in programming, rust, or Godot, but I want to have a go at this issue. What I lack in knowledge, I can make up for in that I have a lot of free time.

Can you give me some directions on how to get started? I run Fedora Workstation 37. I have cargo and godot installed.

This is my first attempt at contributing to a project, so any help is much appreciated ☺️

chitoyuu

chitoyuu commented on Dec 17, 2022

@chitoyuu

@Hanna-Kitten I would suggest starting with the non-generic (Variant) array type if you aren't very experienced with Rust. All you need to write is a bunch of simple wrappers around FFI functions. The VariantArray implementation from GDNative Rust for example looks like this:

https://github.com/godot-rust/gdnative/blob/f9200001af5497bb99f5060bd6c5a97378c8ddf9/gdnative-core/src/core_types/variant_array.rs#L87-L102

You should expect some differences both in the definitions of the FFI functions themselves and in the internal APIs, but the basic idea is the same.

The generic ones require a lot more engineering to get right, so I wouldn't suggest that to a beginner.

Hanna-Kitten

Hanna-Kitten commented on Dec 17, 2022

@Hanna-Kitten

@Hanna-Kitten I would suggest starting with the non-generic (Variant) array type if you aren't very experienced with Rust. All you need to write is a bunch of simple wrappers around FFI functions. The VariantArray implementation from GDNative Rust for example looks like this:

https://github.com/godot-rust/gdnative/blob/f9200001af5497bb99f5060bd6c5a97378c8ddf9/gdnative-core/src/core_types/variant_array.rs#L87-L102

You should expect some differences both in the definitions of the FFI functions themselves and in the internal APIs, but the basic idea is the same.

The generic ones require a lot more engineering to get right, so I wouldn't suggest that to a beginner.

@chitoyuu Thank you so much for your reply. I want to try writing some wrapper functions like you suggested, but I'm not sure what my first objective is.

What should be my first goal/thing to implement (please try to be specific)? With this info I could focus on that idea, and hopefully I can use that knowledge to implement other aspects of this feature set.

I'll need to read about rust/godot FFI/API too. I'm starting from zero in on these subjects. All code I've written to learn is self contained and single purposed and basic in nature.

chitoyuu

chitoyuu commented on Dec 17, 2022

@chitoyuu

What should be my first goal/thing to implement (please try to be specific)? With this info I could focus on that idea, and hopefully I can use that knowledge to implement other aspects of this feature set.

Try running cargo doc on the godot-ffi crate, which I think should give you a listing of all the builtin functions in the GlobalMethodTable type. Look for any functions pertaining to the variant array type, and write wrappers for them if non-existent. If you need to prioritize, start with basic operations like pushing and removing elements.

Focus on the important things: you don't have to do everything all at once. It's likely that the internal API will see a lot of changes before any versioned release, anyway. It could be beneficial, even, to keep the surface small, so less need to be undone when (not if!) the time eventually comes.

I'll need to read about rust/godot FFI/API too. I'm starting from zero in on these subjects. All code I've written to learn is self contained and single purposed and basic in nature.

The Rustonomicon has some information on FFI, but most of the machinery is done for you, so you shouldn't have to worry too much about it. Just call any appropriate *_sys functions whenever you need to convert something from/to its Godot counterpart.

As for Godot FFI particularly, the bad news is that it's severely undocumented. Often you'll have to dig into the engine source code to see if something you want to do is really safe. When in doubt, avoid dealing with lifetimes.

added a commit that references this issue on Jan 25, 2023
f2369b2

29 remaining items

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    c: coreCore componentsfeatureAdds functionality to the librarygood first issueGood for newcomers

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @ttencate@Bromeon@Hanna-Kitten@chitoyuu

        Issue actions

          Array support · Issue #33 · godot-rust/gdext