2

Well, I know this has been asked already and I also happen to know the easiest way to do so.

Now my question is more about your advice on if there is a better way.

I want my method to be called only once when the component is enabled or created. See I can create a component but keep it disabled, then when I enable it for the first time, I want the Init method to be called. The component is contained into an "attached" object.

So I have the Component with

internal bool _runOnce;

then I have the MainObject

List<Component> _listComp = new List<Component>();
void Update(){
   foreach(Component c in _listComp){
      if(!c.enable)continue;
      if(c._runOnce){
          c.Init();
          c._runOnce = false;
      }
      c.Update();
   }
}

My main concern is that the check for _runOnce will happen every frame for every component on each object. I know it is just a boolean check and is worth nothing but I am just asking if anyone would know a pattern for this purpose that is better than this.

Thanks

Everts
  • 10,408
  • 2
  • 34
  • 45
  • 1
    Advice: Don't name public class variables starting with an '_' – Kevin Oct 04 '13 at 18:02
  • @Kevin Me too thought to give same advice but it may be private variable and OP may be in `Component` – Sriram Sakthivel Oct 04 '13 at 18:03
  • @SriramSakthivel If that were the case why would he call it Component and MainObject? – Kevin Oct 04 '13 at 18:04
  • @Kevin I missed that. you're right – Sriram Sakthivel Oct 04 '13 at 18:05
  • Yes it should be internal. – Everts Oct 04 '13 at 18:07
  • Do you always need to run `c.Update();` on ALL components? – Lynn Crumbling Oct 04 '13 at 18:08
  • As for running Update on all components, yes, components are scripts created by the user or some of the factory classes. The Update is required to perform all kind of actions. An Update can be empty, but for the sake of the framework it still has to be called. – Everts Oct 04 '13 at 18:10
  • @fafase According to your code, `update()` does not need to be called if the `Component` is not enabled, but *does* need to be called otherwise (even for run-once `Components`.) Is that correct? – Lynn Crumbling Oct 04 '13 at 18:12
  • Well, the component is created and attached to the GameObject. Then, the GameObject runs the Update of each Component atta0ched to it, if the Component is disabled, no Update is run, all others are run. – Everts Oct 04 '13 at 18:15
  • When you Enable the component, why not call Init then? – Tetsujin no Oni Oct 04 '13 at 18:27
  • Because you can run the component and then disable it but enable it again, that would call Init again. – Everts Oct 04 '13 at 18:28
  • Sure... if `Disable` uninits the object, then the object should set `_isInitialized =false` on itself, and its `Init` should then run again and turn it back true.... If it's not, then `Init` should check `_isInitialized` and return without doing any work... If that becomes too expensive for your update loop, it can be revisited but makes for cleaner easier to read code in your update loop. – Tetsujin no Oni Oct 04 '13 at 18:35
  • 1
    If you want something to happen only once in the lifetime of an object you need some code that is actually related to the lifetime of the object. The only candidates are the constructor or maybe an event that gets fired on creation. Since you don't want to initialize before the component is enabled for the first time all possible candidates have already been executed. Hence you must keep track of initalization yourself (using the `_runOnce` field). The only choice left is **when** to do the check: on **each update** or on **each enabling** of the component. – pescolino Oct 05 '13 at 00:16
  • @pescolino, you should turn that into answer if you want the upvote. I realize what you say totally make sense. I could have the enable boolean to be a property that trigger the init using the same _runOnce principle in the property hence calling only when the component gets enabled. Thanks – Everts Oct 05 '13 at 08:27
  • i am thinking the same question but seem like no actual answer – V-SHY Mar 22 '14 at 13:38

4 Answers4

2

You could also make a list of only the components thats are enabled....

List<Component> _listEnabled = _listComp.Where(item => (item.enable == true));

foreach(Component c in _listEnabled ){
   if(c._runOnce){
      c.Init();           // if at all possible, _runOnce should be set to false
      c._runOnce = false; // IN THE OBJECT, after calling Init();
   }
   c.Update();
}
Lynn Crumbling
  • 12,985
  • 8
  • 57
  • 95
  • Wouldn't Linq make everything slower? I would guess the first line is within the method so it means I get Linq every frame for every object which does not sound like a good idea. If you meant to have it outside the class, then it would miss all non yet created components. Or am I missing something? – Everts Oct 04 '13 at 18:08
  • Linq's going to create a second list of only `Components` that are enabled. There's no sense having them be part of the loop, if you're going to just `continue` and not even need to update() them anyway. – Lynn Crumbling Oct 04 '13 at 18:10
  • I'm just scared that if I have hundreds objects that may hurts the speed. I will still try it and see what it gives. I would think this is suitable for a program where speed is not so much of an issue, I am dealing with a game so it might not be suitable. – Everts Oct 04 '13 at 18:13
  • @fafase I just realized something... if the flag _runOnce is supposed to only allow a Component to run a single time, shouldn't it set `enable` to false after running it? – Lynn Crumbling Oct 04 '13 at 18:17
  • Not to run once, just to call Init once. _runOnce is linked to the Init method and nothing else. – Everts Oct 04 '13 at 18:30
  • 1
    The _listEnabled filter would be better performance-wise if it was using deferred-evaluation .... but would then equate to doing `if (!c.enable)continue;` – Tetsujin no Oni Oct 04 '13 at 18:31
  • `Find` does not return a list. It returns the first element that matches the condition. It should be `Where`. And also: why do you think that the check is faster then? It still does the check for each element. This only adds some extra work to create another list. – pescolino Oct 05 '13 at 00:02
  • @pescolino Wow... how the heck did nobody else see the Find() bug? Ugh. Thanks for the catch. As far as optimization, I'd be curious to benchmark processing a pre-filtered list vs. stepping through each element in a complete list and branching if the condition isn't met. I'm going to theorize that both methods are going to produce nearly the same cycle-time until you hit a sufficiently large list, at which point, preprocessing wins. But again, I'd love some actual benchmark data. – Lynn Crumbling Oct 07 '13 at 13:37
1

I would question whether Update() is the appropriate lifecycle design to Init() your dynamic components.

It seems that it would make more sense to have a method notionally called GameObject.ActivateComponent(AbstractComponent C) call C.Init(), and AbstractComponent::Init call OnInit, which is what the components override and implement. AbstractComponent::Init would make the _runonce check and early-return. It's a method call, but it makes the code more abstract, and has the option to be extended to have a OnReinitialize code path later if you need to provide hooks for 'this is the second or later time I was init'd'. (Say, a statistics reset option...)

It definitely seems wrong for Update() to be probing an implementation detail like "runonce" to find out about the initialization state of Component.

List<Component> _listComp = new List<Component>();
void Update(){
   foreach(Component c in _listComp){
  c.Update();
   }
}

AbstractComponent

 public bool Enable { get;set;}
 private bool _initialized = false;

 void Update(){
     if (!Enable) return;
     Init();
     OnUpdate();
 }

 protected virtual void OnUpdate()
 {
 // filled in by user script
 }

 private void Init()
 {
 if (_initialized) return;
 OnInit();
 _initialized = true;
 }

 protected virtual void OnInit()
 {
 // filled in by user script
 }
Tetsujin no Oni
  • 7,300
  • 2
  • 29
  • 46
  • also note that what I'm calling `AbstractComponent` may be implemented as `Component` in your game model – Tetsujin no Oni Oct 04 '13 at 18:33
  • I was about to say, yes it is. Component could be an abstract class from which user created script are inheriting. As for : "and has the option to be extended to have a OnReinitialize code path later if you need to provide hooks for 'this is the second or later time I was init'd'" There will not be any second init. That is the point of this, only called once at the beginning, never again. – Everts Oct 04 '13 at 18:36
  • editing to add a notional design that moves this responsibility where it belongs.... – Tetsujin no Oni Oct 04 '13 at 18:41
  • Thanks for your time.Happens to be that your second proposed answer is pretty much rewriting what I already have. Thanks anyway. – Everts Oct 04 '13 at 18:59
  • Until profiling showed it was the wrong answer, I'd probably stick with that design then... Optimizing from what I expect on minute inspection to be callvirts versus local field lookups seems like it'd not make things better, but it'd take profiling of a real scenario to be sure. – Tetsujin no Oni Oct 04 '13 at 19:10
1

My main concern is that the check for _runOnce will happen every frame for every component on each object.

From that I assume that you call Update very frequently. My concerns would be for the Update method of the components which is very likely to be much more expensive than a boolean check. This is called micro-optimization: You put a lot of effort into something that isn't a problem at all.

However, I would suggest to encapsulate initialization. Your MainObject does not need to know anything about it. _runOnce should be a private member of the component and enable should be a property (btw: _runOnce should be initialized to true somewhere). Each time your component is enabled you can check for _runOnce and call the initialization if needed:

public class MyComponent
{
    private bool _isInitialized; // I think this is a better name than _runOnce
    private bool _enable;

    public bool Enable
    {
        get
        {
            return _enable;
        }
        set
        {
            if (_enable == value)
            {
                return;
            }

            if (value == true && !_isInitialized)
            {
                Init();
            }

            _enable = value;
        }
    }

    private void Init()
    {
        // initialization logic here ...

        _isInitialized = true;
    }
}

Another idea would be to defer initialization to the Update method. This is basically what you already have but in an object oriented design:

public void Update()
{
    if (_enable && !_isInitialized)
    {
        Init();
    }

    // update logic here ...
}
pescolino
  • 3,086
  • 2
  • 14
  • 24
  • I realized your answer is not fixing the issue. Either I used the property pattern and that fixes the issue when the activation is set for later but if the component is meant to be active right from creation then I am back with initial setup. Or I have to make the call from the Component ctor which does not sound suitable... – Everts Oct 07 '13 at 09:14
  • @fafase: If the component is active right from creation the value of the property will still be set to true somewhere. As long as it is set using the property (not the backing field) the initialization will be called anyway. – pescolino Oct 07 '13 at 15:53
0

What about just a regular constructor for Component?

public Component()
{
   Init();
}
Derek
  • 7,615
  • 5
  • 33
  • 58
  • Because a Component can be created but left disabled. This way it would call the method on creation. See it is for a game object, so you may have all kind of Component attached to the GO, but maybe some of them are just waiting and are inactive. – Everts Oct 04 '13 at 18:16
  • Well you can add the check for enabled in the constructor... but I'm confused.. can the component start disabled and then become enabled? Then what triggers the call to init? – Derek Oct 04 '13 at 18:19
  • Yes the Component can be attached to the GameObject as a setup but kept inactive until something wakes it up. That is when the Init should be called and only then. No more later. – Everts Oct 04 '13 at 18:31
  • then shouldn't the 'wake it up' operation call `Init()`, and not `Update()`? – Tetsujin no Oni Oct 04 '13 at 18:33
  • In this case, I would have an Init method in the GO that does pretty much the same thing happening in the Update, checking for the boolean and then run if necessary, just somewhere else. – Everts Oct 04 '13 at 18:38