assume_safe projection macro for convenient access to sub trees

[chitoyuu]

While looking at #977 I realized that it can be cumbersome to access a fixed scene tree with many moving parts. The situation is similar with what happens with Pin in low level async code. There, the answer is projection as seen in the pin_project crate.

Similarly, we can implement a macro that allows a similar pattern, with projection being the default for convenience:

#[gdnative::project]
struct SubNodes {
    foo: Ref<Node>,
    bar: Ref<Node>,
}

fn do_thing_with_sub_nodes(sub_nodes: &SubNodes) {
    let sub_nodes = unsafe { sub_nodes.assume_safe() };
    let foo: TRef<Node> = sub_nodes.foo;
    let bar: TRef<Node> = sub_nodes.bar;
    // ...
}

All the TRefs generated will have the same lifetime, but this should rarely be an issue for the intended use case of static sub-trees.

Later, this can also be used as a building block for something like #758 (comment)

#[gdnative::fetch] // Unsure about the naming
#[gdnative::project]
struct SubNodes {
    #[fetch(path = "foo")]
    foo: Ref<Node>,
    #[fetch(path = "foo/bar")]
    bar: Ref<Node>,
}

fn do_thing_with_root_node(node: TRef<Node>) {
    let sub_nodes = SubNodes.fetch(node).expect("all nodes are there");
    let sub_nodes = unsafe { sub_nodes.assume_safe() };
    let foo: TRef<Node> = sub_nodes.foo; // ${node.get_path()}/foo
    let bar: TRef<Node> = sub_nodes.bar; // ${node.get_path()}/foo/bar
    // ...
}

I believe this to be a better approach than putting get_node attributes directly in the NativeClass for the flexibility it provides: the API in this proposal allows access to known sub trees with non-Rust roots, while the other one does not. This might not sound hugely important, but considering the use cases of:

• Fetching autoloads (GDScript singletons) from the absolute root node.
• Using in a modding interface where the sub-trees come from third parties.
• Re-using the same structures and composing them for different types, for lack of inheritance.

I think it's pretty obvious that separate types work better.

[chitoyuu]

With a safety redesign being planned, the projection part might quickly become obsolete. A sub-tree fetching macro should always be useful, though.

[Bromeon]

I believe this to be a better approach than putting get_node attributes directly in the NativeClass for the flexibility it provides: the API in this proposal allows access to known sub trees with non-Rust roots, while the other one does not. This might not sound hugely important, but considering the use cases of [...]

I think it's pretty obvious that separate types work better.

The main disadvantage of this API vs. annotations directly in the #[derive(NativeClass)] struct is that an extra type needs to be defined, which might not be necessary in most cases, where a user just wants to model his scene tree in Rust. If that truly becomes a problem, it's possible to also allow these annotations on the exported class directly:

#[derive(NativeClass)]
#[gdnative::project]
struct MyScene {
    child: Ref<Node>,
    anim: Ref<AnimationPlayer>,
}

equivalent to:

#[gdnative::project]
struct Projected {
    child: Ref<Node>,
    anim: Ref<AnimationPlayer>,
}

#[derive(NativeClass)]
#[gdnative::project]
struct MyScene {
    tree: Projected // not sure how this should look, what about the lifetime?
}

[chitoyuu]

an extra type needs to be defined, which might not be necessary in most cases ...

Sure, but a new type like that also costs only around +3 lines in Rust. It can also improve readability by helping code organization when it comes to cases like the example I mentioned in OP:

gdnative/examples/godot_tps_controller_port/src/player.rs

Lines 31 to 59 in a2a85e1

	// Always present sub-nodes
	#[derive(Debug, Clone)]
	struct SubNodes {
	animation_tree: Ref<AnimationTree>,
	player_model: Ref<Spatial>,
	shoot_from: Ref<Position3D>,
	color_rect: Ref<ColorRect>,
	crosshair: Ref<TextureRect>,
	fire_cooldown: Ref<Timer>,
	// Handles Y rotation (yaw)
	camera_base: Ref<Spatial>,
	camera_animation: Ref<AnimationPlayer>,
	// Handles x rotation (pitch)
	camera_rot: Ref<Spatial>,
	camera_camera: Ref<Camera>,
	// Not used in this example, but would be useful to have access to.
	_sound_effects: Ref<Node>,
	sound_effect_jump: Ref<AudioStreamPlayer>,
	sound_effect_land: Ref<AudioStreamPlayer>,
	sound_effect_shoot: Ref<AudioStreamPlayer>,
	shoot_particle: Ref<Particles>,
	muzzle_particle: Ref<Particles>,
	}

...whose respective NativeClass type is already rather complex:

gdnative/examples/godot_tps_controller_port/src/player.rs

Lines 64 to 88 in a2a85e1

	pub struct Player {
	// VARIABLES
	airborne_time: f32,
	orientation: Transform,
	root_motion: Transform,
	motion: Vector2,
	velocity: Vector3,
	aiming: bool,
	// If `true`, the aim button was toggled on by a short press (instead of being held down).
	toggled_aim: bool,
	// The duration the aiming button was held for (in seconds).
	aiming_timer: f32,
	camera_x_rot: f32,
	// ONREADY VARIABLES
	initial_position: Vector3,
	gravity: Vector3,
	// SUB-NODES
	sub_nodes: Option<SubNodes>,
	}

The code organization would be a lot worse if we're forced to mix those fields together.

it's possible to also allow these annotations on the exported class directly

That would be the default case unless explicitly disallowed, no? There are however more challenges when it comes to mixing fields together. The example here looks pretty, but consider the case where the user also needs to export some property and keep some internal state here:

#[derive(NativeClass)]
#[gdnative::project]
struct MyScene {
    #[property]
    #[bikeshed(skip)] // We can't just go by `Ref<_>` because these might be resources
    child_template: Ref<PackedScene>,

    #[property]`
    #[bikeshed(skip)] // ...or references to other nodes set at run-time
    other: Option<Ref<Node>>,

    #[bikeshed(skip)]
    internal_state: MySceneInner,

    child: Ref<Node>,
    anim: Ref<AnimationPlayer>,
}

Suddenly it isn't as pretty anymore because we now have to add extra annotations on everything else.

not sure how this should look, what about the lifetime?

The original type would stay untouched ('static). With GATs now stablized we can make the generated projected type accessible through a trait, without making the type itself namable:

trait BikeshedProject: 'static {
    type Target<'a>: 'a;
    fn fetch(root: TRef<'a, Node>) -> Result<Self::Target<'a>, BikeshedError>;
    fn claim(from: Self::Target<'a>) -> Self; // Using current terminology.
    unsafe fn assume_safe(&self) -> Self::Target<'_>;
}

#[gdnative::project]
struct Projected {
    child: Ref<Node>,
    anim: Ref<AnimationPlayer>,
}

const _: () = {
    use super::*;

    pub struct SomeGeneratedNameToAvoidCollision<'a> {
        pub child: <Ref<Node> as BikeshedProject>::Target<'a>, // TRef<'a, Node>
        pub anim: <Ref<AnimationPlayer> as BikeshedProject>::Target<'a>, // TRef<'a, AnimationPlayer>
    }

    impl BikeshedProject for Projected {
        type Target<'a> = SomeGeneratedNameToAvoidCollision<'a>;
        // ...snip...
    }
}

The projected type could then be named from outside as <Projected as BikeshedProject>::Target<'a>, if truly necessary.

[chitoyuu]

This is of course all assuming that we still want assume_safe to be a thing after #808. It could theoretically exist as an escape hatch for those who don't want any of the runtime checks, but codegen would be complicated because checking on Deref is not enough to ensure safety with the possibility of making multiple TRefs by cloning, or simply Deref-ing and keeping.

A radical idea would be to represent Godot classes as traits instead, which can then be implemented generically for the reference types, but that will force dyn ClassName everywhere and I'm not sure if that's ideal. Technically what we do when the functions are called is dynamic, so perhaps it makes sense, but dyn Trait also has a specific meaning (fat pointer) and we aren't using that, so it could bring some confusion too. I guess some experimentation will be necessary before a method is decided on.