BufferKeyMap macro

[mxgrey]

This PR adds the BufferKeyMap macro to the buffer_map branch.

With #[derive(BufferKeyMap)] users can easily define structs of buffer keys which can then be used with Builder::listen and Builder::create_buffer_access. These structs also support Builder::try_listen and Builder::try_create_buffer_access, which will make listen and buffer access operations much more reasonable to support in the diagram module.

These changes also involve some refactoring of the macros/src/buffer.rs so I'm splitting it out as its own PR to make it easier to review.

Note that I introduced the bevy_impulse::re_exports module so we don't need a dependency on bevy_ecs inside of bevy_impulse_derive since that could lead to problems if the user's crate doesn't have a dependency on bevy_ecs in their Cargo.toml.

[mxgrey]

There's some feedback in #52 that I haven't addressed yet which may also be relevant to this PR. I'll address that feedback after this PR is merged to minimize merge conflicts.

[koonpeng]

With this, for TestJoinedValue, there will be

1. A generated buffers struct
2. A self defined keys struct
3. A generated buffers struct for the keys struct, which may not be compatible as the generated buffers struct, and may not be able to key a TestJoinedValue.

Couldn't we generate the keys struct as well? Then we can impl Accessed on the generated buffer struct and use TestJoinedValue::select_buffers to listen.

[mxgrey]

I expect joining and buffer accessing to be used under different circumstances in many cases. A user may want to join some values together but not care about accessing the buffers or vice versa for different circumstances.

In the case of these tests, I'm just recycling the same set of buffer types because it gives us good test coverage. Even if we could generate all the things from one struct, I'd still want to test each of these macros (JoinedValue and BufferKeyMap) separately.

I'm not opposed to generating everything from a single struct, but it opens up some thorny questions about what the right structure would be for that. There are many different ways we could consider doing it:

• Have the user specify the Buffers struct and then generate the JoinedValue and BufferKeyMap from that
• Have the user specify the JoinedValue struct and then generate the Buffers and BufferKeyMap based on the JoinedValue fields plus attributes. The BufferKeyMap would only be generated if they specify #[joined(buffer_key_map = <name>)].
• Have the user specify the BufferKeyMap and then generate the Buffers and JoinedValue fields. The JoinedValue would only be generated if they specify #[key(joined_value = <name>)].
• Allow the user to choose from any of the above.

For now I'd rather focus on putting out an MVP. We can easily revisit this later and add the above capabilities in a backwards-compatible way after we've gotten some mileage with these features.

[koonpeng]

Similar to how a user will probably get annoyed if they have to maintain both the JoinedValue and _Buffers struct, I think they will get annoyed if they have to maintain a _Keys struct as well. I think the most intuitive is to define the JoinedValue struct and have the others be generated. Doing so also ensures that the _Keys struct would always have the correct key type to the _Buffers struct.

afaik, we need the _Keys struct to impl Accessed, which is needed for join and access operations, I don't think there is any issue with always generating it. There is no cost if the user is not doing anything that requires the _Keys struct, other than arguably increased compilation time, but then if they are concerned about that, they probably shouldn't use derive at all.

From what I understand, it shouldn't be a large change to generate everything from JoinedValue, I can try to push a commit if you think that is a good idea.

[mxgrey]

I think the most intuitive is to define the JoinedValue struct and have the others be generated.

I get what you're saying, but it's not congruent with the intended user experience of each macro.

When I do

#[derive(JoinedValue)]
struct MyJoinedValues { ... }

it's because some time later I want to do

fn my_joined_system(
    In(values): In<MyJoinedValues>,
    ...
) {
    ...
}

And then when I do

#[derive(BufferKeyMap)]
struct MyKeys { ... }

it's because some time later I want to write a listening system:

fn my_listening_system(
    In(keys): In<MyKeys>,
    ...
) {
    ...
}

or perhaps an accessing system:

fn my_accessing_system(
    In((value, keys)): In<(SomeValue, MyKeys)>,
    ...
) {
    ...
}

In all of these cases, the user defines the struct which gets used as an input argument, and then they apply JoinedValue or BufferKeyMap depending on how that struct is being used. I think it would be a bit wonky if the user has to do the following:

#[derive(JoinedValue)]
#[joined(buffer_key_map = MyKeys)]
#[allow(unused)]
struct MyJoinedValues {
    ...
}

fn my_listening_system(
    In(keys): In<MyKeys>,
    ...
) {
    ...
}

They would have to define a MyJoinedValues struct which goes unused in order to obtain the MyKeys struct that they actually want as input to their system.

In some ways this circles back to a concern you raised early on about direct use of auto-generated structs. I agree with your initial concern that the golden path for use cases should be that users do not touch anything that was auto-generated. With two separate macros we'll never put users in a position where they need to directly use an auto-generated struct.

I think it's not actually going to be a common situation in the real world that a user wants to join some set of values with a set of field names and also wants to get bunch of keys to buffers of those same exact types of values with the same exact field names. It happens immediately in our test code because it happens to be convenient for test coverage for the tests of both macros to look similar, but it's not something I expect to see often in real life, i.e. close to never.

I would suggest that we exercise the macros as they currently are and then see if we run into real-world scenarios where someone wants all of these generated from a single source of truth. Then with the benefit of those use cases we can narrow down the right way to support it.

[koonpeng]

I think the issue of having to reference generated struct can be somewhat mitigated by introducing a trait and type alias so you can do

#[derive(JoinedValue)]
#[allow(unused)]
struct MyJoinedValues {
    ...
}

fn my_listening_system(
    In(keys): In<BufferKeys<MyJoinedValues>>,
    ...
) {
    ...
}

But I think renaming the struct and referencing it directly is valid as well. I can see there might be cases where you want a struct of keys without any equivalent struct of joined values, and you don't want to use tuples. So a derive BufferKeyMap will be needed in those cases, but I don't think it hurts if the user has the option to not have to manage multiple structs if for whatever reason they need both a joined value and its equivalent keys struct.

[mxgrey]

I agree in principle with what you're saying about having a single source of truth for all the structs, but I think the occurrence of that use case will be rare, and it's not clear to me what the user will actually want. For example maybe they would want to define the buffer struct as their source of truth and then generate the joined value and buffer key map based off of it. It all depends on why they happen to need that same structure for both joining and for access.

We know what the use cases are for joining and we know what the use cases are for buffer access, but I don't yet know what the use case is for sharing the same structure across both. I agree that it wouldn't be difficult to support what you're saying, my concern is around API stability and consistency. I'll be happy to revisit this in the future when we find a use case for syncing all of these structs.

Until then I'd suggest we move forward with what we know we need and not over-commit for use cases we aren't yet familiar with. It will be less disruptive to add support for the use case later when we can see it and understand it rather than claw back something that we did while speculating about the use case.

In the meantime no one is blocked from doing anything they might need to do with these features.

[koonpeng]

I'm going ahead and merge this so I can move forward with the diagram impl.