Message command context

[vulcandragi]

Hello, I was scheduled these days and something came to my mind that I believe would be a good addition

A way to have a struct that is initially defined in the before command and goes through all the states, the command check, the command itself and the after command

Something similar to how the bot's global data is set

https://github.com/kangalioo/poise/blob/0fc5d548e32fc7e3c8ea490afe95db00ec17ec24/examples/framework_usage/main.rs#L142-L148

So something like that would be in place of the current pre command

https://github.com/kangalioo/poise/blob/0fc5d548e32fc7e3c8ea490afe95db00ec17ec24/examples/framework_usage/main.rs#L117-L121

I believe that there would have to be a change in how the command context is currently defined, to have the definition of the type that would transit between the command states

https://github.com/kangalioo/poise/blob/0fc5d548e32fc7e3c8ea490afe95db00ec17ec24/examples/framework_usage/main.rs#L13

[kangalio]

Another user data field, but initialized and specific for each command invocation separately? Interesting. What would you need this for?

[vulcandragi]

Yes that's right

This would be useful for example to pre-command to do some action in the database to get configs from the guild/user or also to get some cached data referring to the user in question, I believe it would be useful in this sense of specific configs for this case

Or something like there was an error in the internal actions of the command but which is not necessarily an error, and then in the pos command it could be treated and sent to the user something formatted in the embed or even some action in the database, but that would be for more complex things

And also avoid calls to the database, in case you need some data related to the user in the 3 states of the command

[kangalio]

Hmm. I'd like to avoid adding an additional global type parameter like error or user data, it would cause massive churn. An alternative would be a &dyn Any field, which can be accessed with .downcast_ref::<T>().unwrap(). What do you think of that solution?

Also, for the two use cases you mentioned, what do you think of these workarounds?:

• caching data required in multiple command execution phases: using a global in-memory cache for caching user data? Also has the advantage to cache across multiple command invocations to save even more time
• give non-critical errors to post_command to handle: propagating non-critical errors as normal errors, but marking them as non-critical (via enum variant if you have an enum error, or a designated error type if you have a blanket error type like anyhow or Box<dyn Error>) and doing the non-critical error handling in on_error?

[vulcandragi]

I think the way you put it is interesting, that .downcast_ref::<T>().unwrap() would be in ctx ?

The part of the cache I didn't understand very well how it would be, but for example I would use the Data defined globally for this cache would that be?

I hadn't really thought of that in the error handling part, I'll use this

[kangalio]

I think the way you put it is interesting, that .downcast_ref::().unwrap() would be in ctx ?

I don't understand the question but maybe this example code helps

ctx.set_command_data(5);
let five = *ctx.command_data::<i32>().unwrap();

ctx.set_command_data("hi");
let hi = *ctx.command_data::<&str>().unwrap();

Playground link

The part of the cache I didn't understand very well how it would be, but for example I would use the Data defined globally for this cache would that be?

Yes. For example with a HashMap<UserId, YourCachedData> field in Data.

[vulcandragi]

ctx.set_command_data(5);
let five = *ctx.command_data::<i32>().unwrap();

ctx.set_command_data("hi");
let hi = *ctx.command_data::<&str>().unwrap();

Yes, that was my question, I think this addition would be very interesting

And thank you very much for clearing my doubt from the cache that you had commented on.

[kangalio]

Would the API with set_command_data and command_data work for you? If yes, I can implement it

[vulcandragi]

yes it would work

[kangalio]

Done

Implementation involved a bit of jank but it's ok

Feel free to test the changes by changing the dependency branch to "develop" temporarily in your Cargo.toml (cargo docs)

[vulcandragi]

Hello I did some very shallow tests and I liked it a lot, I believe it needs a little polishing on how it works but maybe I can even help with that, this weekend I won't have much time with it but I believe that next week I can help and also do some tests further

[kangalio]

Thank you!

I believe it needs a little polishing on how it works but maybe I can even help with that

Yeah, specific ideas would be much appreciated

[vulcandragi]

I believe I could use for example the tokio mutex using async await

[vulcandragi]

https://github.com/kangalioo/poise/blob/c9d81eb567d9795c643ea076d9626d8ebb609bf3/src/structs/context.rs#L334-L335

I was reading the lib to help, and I came across your comment about parking lot, and internally tokio uses parking lot so I think it wouldn't have as much additional problem as dependency

https://github.com/tokio-rs/tokio/blob/702d6dccc948b6a460caa52da4e4c03b252c5c3b/tokio/Cargo.toml#L103

[kangalio]

Thanks for helping!

I just changed the std mutex to the tokio mutex as you suggested (56ce4fe), so we don't need to consider parking_lot anymore

[kangalio]

Can this issue be closed?

[vulcandragi]

I believe so, I liked the implementation to my tests apparently it worked well, in case I can help in the future and just let me know

[kangalio]

Ok thanks for your feedback!

EDIT: invocation_data (just putting this here to make it easier to search for this issue)