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

Extensive #[func] testing (GDScript -> Rust calls) #200

Closed
wants to merge 4 commits into from

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Mar 22, 2023

Generates both ptrcall and varcall tests now.
Adds several values to the test input.

Currently fails in some ptrcall cases; possibly related to #138 and #195.

@Bromeon Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels Mar 22, 2023
@bors

This comment was marked as outdated.

@Bromeon Bromeon force-pushed the feature/func-tests branch from 9db7baf to 64b764e Compare March 22, 2023 22:41
@Bromeon

This comment was marked as outdated.

bors bot added a commit that referenced this pull request Mar 22, 2023
@Bromeon Bromeon changed the title More tests about #[func] GDScript <-> Rust calls Extensive #[func] testing (GDScript -> Rust calls) Mar 22, 2023
@Bromeon Bromeon force-pushed the feature/func-tests branch 2 times, most recently from d141a79 to 08e8086 Compare March 22, 2023 22:49
@bors

This comment was marked as outdated.

@Bromeon
Copy link
Member Author

Bromeon commented Mar 22, 2023

bors try

bors bot added a commit that referenced this pull request Mar 22, 2023
@bors
Copy link
Contributor

bors bot commented Mar 22, 2023

try

Build failed:

@Bromeon Bromeon force-pushed the feature/func-tests branch 2 times, most recently from 92a3d82 to 7900641 Compare March 22, 2023 23:43
@lilizoey
Copy link
Member

lilizoey commented Mar 23, 2023

seems some of these issues are also related to #195

@Bromeon Bromeon force-pushed the feature/func-tests branch from 7900641 to 310eab7 Compare March 23, 2023 18:52
@Bromeon Bromeon force-pushed the feature/func-tests branch from 0ff64f8 to f669b52 Compare March 26, 2023 15:48
@Bromeon Bromeon force-pushed the feature/func-tests branch from f669b52 to 835ac5d Compare March 26, 2023 15:52
bors bot added a commit that referenced this pull request Apr 12, 2023
204: Fix various issues with pointer calls r=Bromeon a=sayaks

Combines #165 and #200 with more complete fixes.

# Overview

Properly clones strings and shares arrays/dictionaries in pointer calls to ensure they're not prematurely freed.
Frees the value currently in the `ret` pointer in pointer calls, to avoid memory leak.
Moves `ret_val` into `ret`, to avoid premature destruction.
Changes integer conversions in pointer calls to use `as`, since that seems like the right way of converting the values we get from godot. `try_from` lead to wrong conversions sometimes.
Objects inheriting from `RefCounted` now use `ref_get_object` and `ref_set_object` in virtual method calls. 

# Details

Add `from_arg_ptr` and `move_return_ptr` to `GodotFfi` that will be used when converting argument pointers in pointer calls, and moving things into a pointer.
Add `CallType` so `from_arg_ptr` and `move_return_ptr` can have different behavior depending on whether it's a virtual method call or not.
Remove `write_sys` in `GodotFfi`.
Override `from_arg_ptr` in `GodotString`, `NodePath`, `StringName`, `Array`, `Dictionary`, `Gd`.
Override `move_return_ptr` in `Gd`.
Rename `try_write_sys` to `try_return`, and have it use `move_return_ptr` instead of `write_sys`.
Rename `try_from_sys` to `try_from_arg`, and have it use `from_arg_ptr` instead of `from_sys`.
Add `u8`, `u16`, `u32` to the ptrcall tests.
Add a couple of test of virtual methods.
Fix a test for virtual method with return values to actually call the virtual method as a virtual method.

closes #195 
closes #189 



Co-authored-by: Jan Haller <[email protected]>
Co-authored-by: thebigG <[email protected]>
Co-authored-by: Lili Zoey <[email protected]>
@Bromeon
Copy link
Member Author

Bromeon commented Apr 12, 2023

Merged as part of #204.

@Bromeon Bromeon closed this Apr 12, 2023
@Bromeon Bromeon deleted the feature/func-tests branch April 12, 2023 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: register Register classes, functions and other symbols to GDScript feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants