Skip to content
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

Implement Transferable for anything that is Serialize + Deserialize #320

Closed

Conversation

hgzimmerman
Copy link
Member

@hgzimmerman hgzimmerman commented Jul 12, 2018

Anything that is Serialize + Deserialize<'de> will implement Transferable. Implementing Transferable will mark the struct or enum as a valid agent input or output type.

I think either this or #319 should be merged, as either broadens the set of types that can be sent between Agents and the rest of an application. Either of these PRs should allow types defined in other crates to be be Agent::Input or Agent::Output as long as they implement the Serde traits without having to "newtype" them. This allows () to be Input or Output, which is nicer than having to define a Void empty-struct that implements Transferable to represent the same thing.

The particular drawback of this change is that if you have a type that should not be an agent input or output, but is Serialize + Deserialize<'de>, it would be allowed as an Input or Output. As far as I'm aware, things that shouldn't be sent (like tasks) don't implement the Serde traits, so this wouldn't allow any behavior that is currently prohibited for safety reasons. Going on, the inability to implement Serialize + Deserialize<'de> for non-agent-message-same types would remain as a design constraint.

This was referenced Jul 12, 2018
@therustmonk
Copy link
Member

@hgzimmerman I often think about it. It's reasonable, but I used Transferable trait for two reasons:

  1. actix uses similar approach and I think to use actix-core in the future
  2. it may need more restrictions on the future. The trait can help us

But, maybe, it's useful to remove Transferable for now. Keep this PR open, I will match it with a potential moving to actix and let you know.

@hgzimmerman
Copy link
Member Author

I wasn't aware about the use of something like the Transferable trait originating in actix, and maintaining parity with that makes a great deal of sense if that is ultimately the direction that Yew is moving towards.

I do think that because of how minor the complaint that these PRs addresses, the similarity to actix trumps the small ergonomic benefit proposed here.

@kellytk
Copy link
Contributor

kellytk commented Sep 20, 2018

@deniskolodin

  1. I think to use actix-core in the future

As I mention in #272 (comment), another option for your consideration is Riker. Regardless of which actor framework you use, I think it's a good idea to converge components with the actor model/ECS.

@jstarry
Copy link
Member

jstarry commented Sep 30, 2019

Closing in favour of #319

@jstarry jstarry closed this Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants