idanarye / woab

Widgets on Actors Bridge - a GUI microframework for combining GTK with Actix
https://crates.io/crates/woab
MIT License
14 stars 1 forks source link

Action routing in the post-signal-overhaul age #21

Closed idanarye closed 3 years ago

idanarye commented 3 years ago

Before #20, actions could be connected with SignalRouter like so:

let router = TestSignal::connector().route_to::<TestActor>(ctx);
// ...
router.connect(&action, "activate", signal_name).unwrap();

And this made sense, since the action signals were converted to a BuilderSignal enum variant.

20 removes BuilderSignal, and while actions can still be connected with:

woab::route_signal(action, "activate", signal_name, ctx.address()).unwrap();

This creates a woab::Signal, which means it takes two steps to get the action parameter - first convert the glib::Value to glib::Variant, and then get the concrete type from the variant.

This is far less convenient than what we had!

Actions are common enough (I think?) and always have the same structure. Why not give them their own message type?

woab::route_action_activate(action, ctx.address()).unwrap();
// Or
woab::route_action_change_state(action, ctx.address()).unwrap();

We can use the action to identify the signal (either by name or by exposing the action object) and the action type can

impl actix::Handle<woab::ActionSignal> for MyActor {
    type Result = woab::ActionSignalResult;

    fn handle(&mut self, msg: woab::Signal, ctx: &mut <Self as actix::Actor>::Context) -> Self::Result {
        Ok(match msg.name() {
            "action1" => {
                let param = msg.param()?; // Unlike woab::Signal::param, woab::ActionSignal.param needs no index
                // ...
            }
            "action2" => {
                // ...
            }
            _ => msg.cannot_handle()?,
        })
    }
}
piegamesde commented 3 years ago

To be honest, I like having all my handlers in one place and only having to implement Handler once. Most of the time, I only care about the action and don't use the parameters anyways.

I'd prefer a solution that makes the boilerplate manageable. I'm thinking of something similar to how we deal with event subtypes. For example, one could add an action_param<T>, that asserts the number of arguments, casts the first argument to Variant and then further down to T or something like that.

idanarye commented 3 years ago

Makes sense. Now that I think about it, my main beef is with the verbosity of woab::route_signal which is unneeded when it comes to actions, but I can have woab::route_action that deduces the signal type and the signal name from the action, and sends a standard woab::Signal.

I was actually thinking about having msg.param_variant(1) for actions and msg.param_event(1) for events, to make things flexible enough for anything that uses variants/events. But I can't really think of any other signal that uses these, and while this doesn't mean there aren't any - one could always use msg.param() and convert it manually, so we are not losing much from using msg.action_param() and msg.event_param() that can only use the second parameter.

Is it really necessary though to fail if there are more than two parameters?

piegamesde commented 3 years ago

Is it really necessary though to fail if there are more than two parameters?

Good question. I'm failing to find any case where implicitly ignoring arguments could lead to undesired behavior.

idanarye commented 3 years ago

OK - action routing looks good now, with woab::route_action() and Signal::action_param(). https://github.com/idanarye/woab/blob/action-routing/examples/example_actions.rs