Action<Posef>::create_space takes a Session instead of a &Session

[Supreeeme]

It seems a little counterintuitive for this to consume a session vs just taking a reference to one, like the other Action associated functions.

[Ralith]

Huh? https://github.com/Ralith/openxrs/blob/e5fe4164216fcc16189c52ea58e639e85e8e2d6b/openxr/src/action.rs#L78-L81

[Supreeeme]

https://github.com/Ralith/openxrs/blob/e5fe4164216fcc16189c52ea58e639e85e8e2d6b/openxr/src/action.rs#L82 Why does this take a Session<G> instead of &Session<G>?

[Ralith]

Oh, right, misread. Because it takes ownership of it; if it took a &Session, it'd have to clone internally.

[Supreeeme]

Why does a space need to take ownership of the session? If you want to create more than one space (or do anything with the session after creating the space really) you end up having to clone the session anyway. And looking at the definition for Space::action_from_raw it only uses the inner session handle (which is Copy) so I don't see a reason to consume it.

[Ralith]

Why does a space need to take ownership of the session?

So that the Drop impl can work.

it only uses the inner session handle (which is Copy)

Arc<session::SessionInner> is not Copy.

[Supreeeme]

Which Drop impl are you referring to? The Space Drop impl (actually the Space as a whole) just relies on SessionInner, not Session.

[Ralith]

SessionInner comes from Session. Hence the name.

[Supreeeme]

if it took a &Session, it'd have to clone internally.

Didn't see you say this. Why prefer an external clone? I can't imagine creating a space is the last thing someone wants to do with a session.

[zmerp]

I'd say having Action<Posef>::create_space take a &Session is less surprising because it is symmetric to other parts of the API.

[Ralith]

Fair points; overwhelmingly callers probably don't have short-lived Session values, so providing the ability to move one in isn't likely to be useful. I'd approve a PR making this change.