-
Notifications
You must be signed in to change notification settings - Fork 2
fix: listeners can now run in tasks #25
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
## Walkthrough
The changes update the lifetime quantifiers in the trait bounds for callback parameters in three asynchronous listener methods within the `Client` implementation. The callbacks now accept two references with independent lifetimes instead of a single shared lifetime. Additionally, the `warframe-macros` crate version was updated from "6.2.0" to "7.0.0" in its `Cargo.toml`, and an extra space was removed in the dependency declaration in the main `Cargo.toml`.
## Changes
| File(s) | Change Summary |
|------------------------------|------------------------------------------------------------------------------------------------------|
| Cargo.toml, warframe-macros/Cargo.toml | Removed extra space before comma in `warframe-macros` dependency; updated `warframe-macros` version from "6.2.0" to "7.0.0". |
| src/worldstate/client.rs | Updated callback trait bounds in three async listener methods to use two independent lifetimes instead of one shared lifetime. No logic or control flow changes. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Caller
participant Client
participant Callback
Caller->>Client: call_on_update(callback)
Client->>Callback: Invoke callback(&'a T, &'b T) with independent lifetimes
Callback-->>Client: Complete
Client-->>Caller: Result<(), Error> |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Cargo.toml (1)
35-35: Consider retaining aversionalongside thepathforwarframe-macros.While
cargoignores theversionfield for localpathdependencies during builds, the field becomes mandatory when you eventually publish the crate to crates.io (path deps are disallowed on publish and must be replaced by a normal dependency). Keeping the version now avoids a future search-and-replace step and documents the intended semver relationship.If you prefer the leaner manifest locally, ensure you have a release checklist entry that restores the version before publishing.
src/worldstate/client.rs (2)
253-256: Good fix – independent lifetimes unblock async tasks, but update the docs to match.Changing
for<'a,'b> Callback: AsyncFn(&'a T, &'b T)removes the “not general enough” error and allows the two references to have unrelated lifetimes. Nice catch.
The surrounding rust-doc comments (lines 220-222) still talk about a single'anylifetime—worth updating for clarity.No further issues spotted.
448-451: Same lifetime tweak correctly applied here; documentation again lags behind.The analogous change to
for<'a,'b> Callback: AsyncFn(S, &'a T, &'b T)solves the same problem for the stateful variant. Just remember to adjust the doc block above (lines 398-400) so readers don’t see the obsolete
'anywording.Implementation looks correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Cargo.toml(1 hunks)src/worldstate/client.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
## [7.0.1](v7.0.0...v7.0.1) (2025-07-06) ### Bug Fixes * listeners can now run in tasks ([#25](#25)) ([c22822c](c22822c))
|
🎉 This PR is included in version 7.0.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What did you fix?
Listeners could not be run in task, especially the "call_on_update" ones.
They would yield an error such as "AsyncFnMut is not general enough".
This fixes that issue.
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
No user-facing features or bug fixes were introduced in this release.