-
-
Notifications
You must be signed in to change notification settings - Fork 403
fix:trait method signature mismatch for plugin_method #1231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
After testing, it can only solve the compilation problem, but it will get stuck during operation 2025-12-08 17:52:33 [INFO] (57) schedule thread start id: ThreadId(57) name: Schedule Thread |
Any ideas why?, Would be great to have that fixed before merging |
| crate::GLOBAL_RUNTIME.block_on(async move { | ||
| #fn_body | ||
| fn #fn_name(#fn_inputs) -> std::pin::Pin< | ||
| Box<dyn std::future::Future<Output = Result<(), String>> + Send> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it is true that all current plugin methods return a Result<(), String>, this may not be the case in the future. I'd personally prefer if the input function's return type was taken into account when generating the final function.
I'm sorry, my abilities are limited. Let's first address the issue with plugin_method |
I have checked it and it seems that the problem is with on_load |
|
The reason is the deadlock caused by |
|
I've done my own investigation into this as well. Since dynamically included librarys have a different address space, the logger never is able to use the already defined logger in the pumpkin main process. Instead it must create its own first, but that is never done, so it just waits, this was exposed in #1201. The best way to fix this might be to remove the macro entirely, and to add the logger to the context instead. I'm working on that in my PR #1241 |
|
fixed in #1241 |
Background
Pumpkin’s Plugin trait requires plugin methods (e.g., on_load) to return:
However, the current implementation of the #[plugin_method] macro rewrites methods into an async fn, which compiles into:
This causes a type mismatch when implementing the Plugin trait, resulting in compilation error E0053.
❗ Problem
When using:
the generated method has signature:
but the trait expects:
This mismatch triggers:
error[E0053]: method
on_loadhas an incompatible type for trait✅ Solution
Modify the plugin_method macro to wrap the async block into a boxed future, producing the correct return type expected by the Plugin trait.
Key Changes
Replace the generated async fn with a regular fn returning:
Wrap the original async body using:
Preserve the user-written method body while ensuring type compatibility.
📄 Summary of Changes
Updated the expansion of #[plugin_method] to generate a boxed future instead of impl Future.
Ensured all plugin methods rewritten by the macro now fully satisfy the Pumpkin Plugin trait signatures.
No changes required from plugin authors—existing macro usage continues to work.
🔍 Impact
Fixes compilation failure for any method tagged with #[plugin_method].
Makes macro behavior trait-compliant.
No breaking API changes.