bevy_reflect: Improve DynamicFunction ergonomics

[MrGVSV]

Objective

Many functions can be converted to DynamicFunction using IntoFunction. Unfortunately, we are limited by Rust itself and the implementations are far from exhaustive. For example, we can't convert functions with more than 16 arguments. Additionally, we can't handle returns with lifetimes not tied to the lifetime of the first argument.

In such cases, users will have to create their DynamicFunction manually.

Let's take the following function:

fn get(index: usize, list: &Vec<String>) -> &String {
    &list[index]
}

This function cannot be converted to a DynamicFunction via IntoFunction due to the lifetime of the return value being tied to the second argument. Therefore, we need to construct the DynamicFunction manually:

DynamicFunction::new(
    |mut args, info| {
        let list = args
            .pop()
            .unwrap()
            .take_ref::<Vec<String>>(&info.args()[1])?;
        let index = args.pop().unwrap().take_owned::<usize>(&info.args()[0])?;
        Ok(Return::Ref(get(index, list)))
    },
    FunctionInfo::new()
        .with_name("get")
        .with_args(vec![
            ArgInfo::new::<usize>(0).with_name("index"),
            ArgInfo::new::<&Vec<String>>(1).with_name("list"),
        ])
        .with_return_info(ReturnInfo::new::<&String>()),
);

While still a small and straightforward snippet, there's a decent amount going on here. There's a lot of room for improvements when it comes to ergonomics and readability.

The goal of this PR is to address those issues.

Solution

Improve the ergonomics and readability of manually created DynamicFunctions.

Some of the major changes:

1. Removed the need for &ArgInfo when reifying arguments (i.e. the &info.args()[1] calls)
2. Added additional pop methods on ArgList to handle both popping and casting
3. Added take methods on ArgList for taking the arguments out in order
4. Removed the need for &FunctionInfo in the internal closure (Change 1 made it no longer necessary)
5. Added methods to automatically handle generating ArgInfo and ReturnInfo

With all these changes in place, we get something a lot nicer to both write and look at:

DynamicFunction::new(
    |mut args| {
        let index = args.take::<usize>()?;
        let list = args.take::<&Vec<String>>()?;
        Ok(Return::Ref(get(index, list)))
    },
    FunctionInfo::new()
        .with_name("get")
        .with_arg::<usize>("index")
        .with_arg::<&Vec<String>>("list")
        .with_return::<&String>(),
);

Alternatively, to rely on type inference for taking arguments, you could do:

DynamicFunction::new(
    |mut args| {
        let index = args.take_owned()?;
        let list = args.take_ref()?;
        Ok(Return::Ref(get(index, list)))
    },
    FunctionInfo::new()
        .with_name("get")
        .with_arg::<usize>("index")
        .with_arg::<&Vec<String>>("list")
        .with_return::<&String>(),
);

Testing

You can test locally by running:

cargo test --package bevy_reflect

---

Changelog

• Removed &ArgInfo argument from FromArg::from_arg trait method
• Removed &ArgInfo argument from Arg::take_*** methods
• Added ArgValue
• Arg is now a struct containing an ArgValue and an argument index
• Arg::take_*** methods now require T is also TypePath
• Added Arg::new, Arg::index, Arg::value, Arg::take_value, and Arg::take methods
• Replaced ArgId in ArgError with just the argument index
• Added ArgError::EmptyArgList
• Renamed ArgList::push to ArgList::push_arg
• Added ArgList::pop_arg, ArgList::pop_owned, ArgList::pop_ref, and ArgList::pop_mut
• Added ArgList::take_arg, ArgList::take_owned, ArgList::take_ref, ArgList::take_mut, and ArgList::take
• ArgList::pop is now generic
• Renamed FunctionError::InvalidArgCount to FunctionError::ArgCountMismatch
• The closure given to DynamicFunction::new no longer has a &FunctionInfo argument
• Added FunctionInfo::with_arg
• Added FunctionInfo::with_return

Internal Migration Guide

Important

Function reflection was introduced as part of the 0.15 dev cycle. This migration guide was written for developers relying on main during this cycle, and is not a breaking change coming from 0.14.

• The FromArg::from_arg trait method and the Arg::take_*** methods no longer take a &ArgInfo argument.
• What used to be Arg is now ArgValue. Arg is now a struct which contains an ArgValue.
• Arg::take_*** methods now require T is also TypePath
• Instances of id: ArgId in ArgError have been replaced with index: usize
• ArgList::push is now ArgList::push_arg. It also takes the new ArgValue type.
• ArgList::pop has become ArgList::pop_arg and now returns ArgValue. Arg::pop now takes a generic type and downcasts to that type. It's recommended to use ArgList::take and friends instead since they allow removing the arguments from the list in the order they were pushed (rather than reverse order).
• FunctionError::InvalidArgCount is now FunctionError::ArgCountMismatch
• The closure given to DynamicFunction::new no longer has a &FunctionInfo argument. This argument can be removed.

[MrGVSV]

Clippy's not happy with my choice of naming in ArgList::next.

Should we rename? If so, to what?

So far on Discord, we've come up with the following alternatives:

• take
• take_next
• take_first
• pull (I think this one is my favorite)
• pop_front (and probably rename pop to pop_back)

Keep in mind that we also need _arg, _owned, _ref, and _mut variants of whatever method name we choose.

We might also want to consider whether we want to keep ArgList::pop or remove it altogether.

[MrGVSV]

I just went with ArgList::take for now. Let me know if there are any other opinions on this!

[alice-i-cecile]

Much improved API; fantastic PR writeup.

[mweatherley]

Some minor docs stuff. Looks like this makes the whole IntoFunction process more straightforward and understandable, which I'm happy about :)