Zinnia.Unity icon indicating copy to clipboard operation
Zinnia.Unity copied to clipboard

Can we merge BehaviourEnabledObserver and RulesMatcher?

Open bddckr opened this issue 6 years ago • 2 comments

BehaviourEnabledObserver definitely should take a Rule instead, at which point it's very similar to RulesMatcher. Perhaps the repeated invoking can be done as a standalone component that repeatedly calls an IProcessable.Process until canceled?

bddckr avatar Mar 10 '19 10:03 bddckr

  • BehaviourEnabledObserver == IProcessable + a new IRule that does a list check for all behaviours being enabled
  • RulesMatcher
  • this new "ProcessCaller"

bddckr avatar Mar 11 '19 10:03 bddckr

Before image

After image

Initial thoughts and decisions involved:

RulesMatcher

RulesMatcher matches a list of rules against the same object (behaviour), whereas BehaviourEnabledObserver matches a single rule (isActiveAndEnabled) against all behaviour in the list. So they are not quite compatible.

a new IRule that does a list check for all behaviours being enabled

IRule interface only Accept(object) a single object. It is incompatible with matching each behaviour in the list. Sounds like a ICollectionRule.Accept(ObservableList).

IsActiveAndEnabledRule

IsActiveAndEnabledRule was the first thing I write. In retrospect, the current BehaviourEnabledObserver2 can use any rule, so it is actually not that specific and more like a BehaviourRuleMatcher. I'm not sure about the difference between RulesMatcher and AllRules but I guess it depends on usage. Anyway, we can keep it specific and check isActiveAndEnabled inside the observer just like the previous version.

bddckr commented that:

The issue now is

  1. Users have to add many more components
  2. All prefabs need to be updated :slightly_smiling_face:

fight4dream avatar Aug 08 '19 10:08 fight4dream