asynchronics / asynchronix

High-performance asynchronous computation framework for system simulation
Apache License 2.0
170 stars 9 forks source link

Implement InputFn for a higher amount of input arguments #17

Closed robamu closed 6 months ago

robamu commented 6 months ago

I had an input function like this:

pub async fn switch_device(&mut self, switch: PcduSwitch, target_state: SwitchStateBinary) {
   ...
}

and had to rewrite it as:

pub async fn switch_device(&mut self, switch_and_target: (PcduSwitch, SwitchStateBinary)) {
   ...
}

Did I do something wrong or is the number of additional arguments constrained to 1? it would be nice if I could instead use the initial version. This probably would be possible by implementing InputFn for a certain number of arguments. I saw a similar thing done inside the bevy game engine to allow magically passing functions with a higher number of arguments to the engine:

  1. Implementation macro for a all functions which can be system functions (analogue to the input functions, see this specific line): https://github.com/bevyengine/bevy/blob/c75d14586999dc1ef1ff6099adbc1f0abdb46edf/crates/bevy_ecs/src/system/function_system.rs#L639
  2. https://github.com/bevyengine/bevy/blob/c75d14586999dc1ef1ff6099adbc1f0abdb46edf/crates/bevy_utils/macros/src/lib.rs#L109 : Helper macro to call this macro for tuple variants.
  3. https://github.com/bevyengine/bevy/blob/c75d14586999dc1ef1ff6099adbc1f0abdb46edf/crates/bevy_ecs/src/system/function_system.rs#L697 macro call to implement this for 0 to 16 arguments if I understand correctly.
robamu commented 6 months ago

This is probably really tricky because a lot of the other API relies on T being one argument. For bevy, there are also constraints on the possible input parameter types, so I don't know how applicable that is for this case.

UPDATE: This little test seems work. So in principle, this probably is possible, but the T argument now becomes a marker trait and the function arguments are specified by an associated argument.. so this is definitely a major breaking change..

sbarral commented 6 months ago

This was considered but ultimately abandoned for a couple of reasons:

sbarral commented 6 months ago

Also, it makes it possible to add other optional arguments than Scheduler in the future.

robamu commented 6 months ago

Sounds reasonable. You can close this issue :+1: