Some general feedback

[piegamesde]

I finally got around to port most of my application to WoAB, and I ought to comment on #12 but it touches a lot of things that are not really specific to that pull request. In random order:

• I need to access the context from an ActorBuilder (or alternatively, the Address) in order to build cross-referencing actors
• To we still need ActorBuilderContext? The only method it seems to provide is widgets, but we already have it on BuilderConnector
• The trait RegisterSignalHandlers is not public? (at least in my documentation)
• I'm not 100% sure about the inhibiting mechanism.
  • I don't like the fact that I have to split my event handling code into two somewhat redundant halves, but I fear that nothing more can be done about this.
  • I added an inhibit method to my signal in order to move that code outside of the main where I do the signal connection, and then call it with .connect_signals(MySignal::connector().inhibit(MySignal::inhibit)).
  • I don't really like the mix-and-match with the Option and default values from the macro
  • This brought me to the idea: maybe add inhibit as an optional method to the BuilderSignal trait? The default implementation can then take the arguments from the macro if present. And if one wants dynamic values, a manual implementation can/must be used.

[idanarye]

• I need to access the context from an ActorBuilder (or alternatively, the Address) in order to build cross-referencing actors

I can add an ActorBuilder::actix_ctx() method - this one is easy enough that I can shove it to version 0.2.

• To we still need ActorBuilderContext? The only method it seems to provide is widgets, but we already have it on BuilderConnector

Yes - for ActorBuilder::create() and ActorBuilder::try_create(). And it's also a Deref and DerefMut to the actor context.

• The trait RegisterSignalHandlers is not public? (at least in my documentation)

It's public with #[doc(hidden)]. And I'm not even sure we need it - I'm thinking of just removing it and have connect_signals just receive a BuilderSingalConnector.

• I'm not 100% sure about the inhibiting mechanism.

  • I don't like the fact that I have to split my event handling code into two somewhat redundant halves, but I fear that nothing more can be done about this.
  • I added an inhibit method to my signal in order to move that code outside of the main where I do the signal connection, and then call it with .connect_signals(MySignal::connector().inhibit(MySignal::inhibit)).
  • I don't really like the mix-and-match with the Option and default values from the macro
  • This brought me to the idea: maybe add inhibit as an optional method to the BuilderSignal trait? The default implementation can then take the arguments from the macro if present. And if one wants dynamic values, a manual implementation can/must be used.

I agree that this got messy and unsightly. Since you have already convinced me to try and see how feasible is it to crank the Tokio runtime inside the signal handlers, I think the best solution - if this is indeed feasible - is to make the the inhibit value the Result of the message (replacing the current mechanisms)

I don't think we can add an inhibit method to the BuilderSignal trait, because this trait is implemented by a macro - so how would the user provide their own implementation?

[piegamesde]

Yes - for ActorBuilder::create() and ActorBuilder::try_create(). And it's also a Deref and DerefMut to the actor context.

I think you misunderstood me. Is there any reason we can't pass the real context to those methods and ditch the extra struct? I don't think the original reason for introducing it (connecting signals and widgets IIRC) does still exist.

I don't think we can add an inhibit method to the BuilderSignal trait, because this trait is implemented by a macro - so how would the user provide their own implementation?

• The method of the trait provided by the trait, so there is no need to implement it
• The macro leaves out that method when generating the impl BuilderSignal
  • If the inhibit is annotated in the macro, the macro will generate an inhibit method based on that information. The struct annotations are mutually exclusive to custom dynamic behavior.
• The user can then add an impl BuilderSignal block containing only that method
• I'm not sure if Rust allows me to do this, but I'm optimistic about this

[idanarye]

20 made all of these obsolete.