Ignore enum variants in BuilderSignal

[piegamesde]

I'm not sure if this is the correct way to use Actix, but at the moment I'm sharing a signal enum for both gtk and other messages. Therefore, I'd like to annotate some variants to be ignored by the macro. They can then contain arbitrary data.

[idanarye]

Not that I'm against adding it, but the Actix way is to just make them messages of their own. The reason WoAB uses enums for signals is that I need to use streams rather than messages to guarantee to the compiler that I'm not passing the GTK objects past thread boundaries, and I don't want to allocate a stream for each signal.

[piegamesde]

Okay, I just stumbled upon the threading issue again. So the problem is that message handlers have the Send requirement? As our execution engine is single threaded, we can go around this by using send_wrapper. Would this then allow to have one message struct per signal?

[idanarye]

But then I lose my comfortable excuse for using enum variants :wink:. This will make things very verbose, which each signal having it's own type, it's own connect_signal call when you create the actor, and with it's own impl Handler. Unlike other Actix users I won't frown at you if you make your own enum for the non-GTK signals, which you can probably send as regular messages because making a new stream for them is too much work. But why send them as part of the GTK signals stream? Doing so would mean you'll also need a way to get the Tokio mpsc::Sender to whatever sends these messages - which means WoAB will also need to provide a way for you to get that sender (more complex syntax) when addresses and recipients are so much easier to get and to pass around.

The main reason to use regular messages instead of streams is that unless you are dealing with an actual stream, regular messages are easier to setup. To send a message all you need is the Addr (or Recipient) which Actix makes very easy to pass around. To connect a stream you need the Context - which you only get when creating the actor and when connecting the actor. But with WoAB you are already connecting the signals when you are creating the actor, and WoAB handles the more verbose syntax of connecting a stream, so as far as the user cares the only difference between a stream and regular messages is that they need to use StreamHandler instead of Handler.

[piegamesde]

Well, there are subtle differences between a Handler and a StreamHandler, for example regarding the return type, but that's not my point. The thing is that I want it to be consistent, because otherwise if I want to manually send some events GTK sends as well, I need to have both a Handler and a StreamHandler for the same signal which is annoying. (The alternative would be to send events directly to the stream, but as far as I understand that would require a Sender instead of an address?

I'll leave this be for now, but my vision is to have a macro that generates all those impl Handler blocks by delegating them to simple functions with the matching signature on impl MyActor. Then, there would be no pain in having one message type per signal.

[piegamesde]

Well, my macro idea is going down the drain, so instead I'm going for enum messages instead (at least in some circumstances). Which again raises the question: How much pain would it be to have the WoAB message handling using a Handler instead of a StreamHandler? Together with a macro extension to "ignore" certain branches, this might be the most ergonomic solution for some of my problems.

[idanarye]

Mmm... This is hard. We are already need to support Handler for #7, but messages need to be Send and GTK objects are !Send. And while most of the !Send is the widget that gets passed as first argument and one usually doesn't need - there are arguments that we do need, like a gdk::Event or a cairo::Context...

You suggested using send_wrapper, but in order to make this transparent to the user I'd have to do something like:

pub SendWrapperWrapper<M: actix::Message>(SendWrapper<M>);

impl<M: actix::Message> actix::Message for SendWrapperWrapper<M> {
    type Result = M::Result;
}

impl<A, M> actix::Handler<SendWrapperWrapper<M>> for A
where
    A: actix::Actor,
    M: actix::Message,
    A: actix::Handler<M>,
{
    // This is the part I'm worried about. Will it even work?
    type Result = <A as actix::Handler<M>>::Result;

    fn handle(&mut self, msg: SendWrapperWrapper<M>, ctx: &mut Self::Context) -> Self::Result {
        let msg = msg.0.take();
        self.handle(msg, ctx)
    }
}

At any rate - I still think you can always just split your messages to two enums - a BuilderSignal enum that you send in a stream and another enum you send as a regular Actix message.

[piegamesde]

And while most of the !Send is the widget that gets passed as first argument and one usually doesn't need

Actually, none of the glib objects or any of its descendants are Send. One can use some of them across threads (excepting the GTK widgets of course), but building a safe Rust API for this is a subject you definitely don't want to get into :)

Your code example actually doesn't look that bad. We could restrict the return type anyways because the GTK callbacks don't have any (except an Inhibit, but lol).

I still think you can always just split your messages to two enum

Yeah, that's what I'm doing at the moment. I think my motivation for having both was some use case where I wanted to re-use one of the variants of the GTK enum to call it from code, but which required me to get a Sender and I only had an Addr. Maybe that's a weird use case anyways.

[idanarye]

Yeah, that's what I'm doing at the moment. I think my motivation for having both was some use case where I wanted to re-use one of the variants of the GTK enum to call it from code, but which required me to get a Sender and I only had an Addr. Maybe that's a weird use case anyways.

I'm not sure my SendWrapperWrapper solution would help you much, since you won't be able to send the message directly (even if your variant is Send, the entire enum is probably !Send). You could probably wrap it yourself, but that's... ugly.