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

Add into_inner method to ArcData #700

Merged
merged 1 commit into from Feb 23, 2021
Merged

Conversation

Waridley
Copy link
Contributor

As discussed in Discord, this method is especially useful for coercing the script of an Instance into a trait object, thus being able to benefit from more Rust conveniences within Godot scripts.

This could be written as an impl<T> Into<Arc<T>> for ArcData<T>, but I feel like this makes it more clear that it's not expected for the user to be able to freely treat ArcData as an Arc whenever they want to, but rather must be explicit about accessing the internals of this type -- even if it should theoretically be as memory-safe as using the ArcData anyway.

I decided to have this method take ownership of self because if you have reference to an ArcData, you should still be able to call clone() on it to get an owned copy, but the opposite would not be as easy if the user didn't want to increment the ref count.

@Waridley Waridley force-pushed the arc_into_inner branch 2 times, most recently from e6988a3 to 36199b0 Compare February 23, 2021 04:48
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

bors r+

bors bot added a commit that referenced this pull request Feb 23, 2021
700: Add into_inner method to ArcData r=toasteater a=Waridley

As discussed in Discord, this method is especially useful for coercing the `script` of an `Instance` into a trait object, thus being able to benefit from more Rust conveniences within Godot scripts.

This *could* be written as an `impl<T> Into<Arc<T>> for ArcData<T>`, but I feel like this makes it more clear that it's not expected for the user to be able to freely treat `ArcData` as an `Arc` whenever they want to, but rather must be explicit about accessing the internals of this type -- even if it should theoretically be as memory-safe as using the `ArcData` anyway.

I decided to have this method take ownership of `self` because if you have  reference to an `ArcData`, you should still be able to call `clone()` on it to get an owned copy, but the opposite would not be as easy if the user didn't want to increment the ref count.

Co-authored-by: Waridley <Waridley64@gmail.com>
@ghost ghost added the feature Adds functionality to the library label Feb 23, 2021
@bors
Copy link
Contributor

bors bot commented Feb 23, 2021

Build failed:

@ghost
Copy link

ghost commented Feb 23, 2021

Failed due to new clippy lints fixed in #703

bors retry

@bors
Copy link
Contributor

bors bot commented Feb 23, 2021

Build succeeded:

@bors bors bot merged commit f2b4080 into godot-rust:master Feb 23, 2021
@Waridley Waridley deleted the arc_into_inner branch April 3, 2021 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant