Entitas icon indicating copy to clipboard operation
Entitas copied to clipboard

Why don't add "Update/Replace" GroupEvent?

Open yuchting opened this issue 7 years ago • 10 comments

Hi,

It's me again, I have a question want to ask. In game reactive system, we have to override the GetTrigger method to collect game entity while some components added or removed, but I cannot see the "Update/Replace" event.

I checked the source code, in Group.cs, there is some code holding the updating event:

        /// This is used by the context to manage the group.
        public void UpdateEntity(TEntity entity, int index, IComponent previousComponent, IComponent newComponent) {
            if (_entities.Contains(entity)) {
                if (OnEntityRemoved != null) {
                    OnEntityRemoved(this, entity, index, previousComponent);
                }
                if (OnEntityAdded != null) {
                    OnEntityAdded(this, entity, index, newComponent);
                }
                if (OnEntityUpdated != null) {
                    OnEntityUpdated(
                        this, entity, index, previousComponent, newComponent
                    );
                }
            }
        }

But, in Collector.cs where the OnEntityAdded and OnEntityRemoved delegate were assigned, there's not any code to set Update/Replace:

        /// Activates the Collector and will start collecting
        /// changed entities. Collectors are activated by default.
        public void Activate() {
            for (int i = 0; i < _groups.Length; i++) {
                var group = _groups[i];
                var groupEvent = _groupEvents[i];
                switch (groupEvent) {
                    case GroupEvent.Added:
                        group.OnEntityAdded -= _addEntityCache;
                        group.OnEntityAdded += _addEntityCache;
                        break;
                    case GroupEvent.Removed:
                        group.OnEntityRemoved -= _addEntityCache;
                        group.OnEntityRemoved += _addEntityCache;
                        break;
                    case GroupEvent.AddedOrRemoved:
                        group.OnEntityAdded -= _addEntityCache;
                        group.OnEntityAdded += _addEntityCache;
                        group.OnEntityRemoved -= _addEntityCache;
                        group.OnEntityRemoved += _addEntityCache;
                        break;
                }
            }
        }

And there is not "Update/Replace" in GroupEvent.cs at all.

So, Is there some design reason or another reason to stop adding "Update/Replace" event in Collector?

Cheers

yuchting avatar Jan 24 '18 09:01 yuchting

Replace and Add are not distinguished in the collectors: e.AddThing(x,y,z) and e.ReplaceThing(x,y,z) will both show up in the reactive system with trigger Matcher.Thing.Added().

FNGgames avatar Jan 24 '18 09:01 FNGgames

@FNGgames thanks for so quick answer, I know that, but if there's not "Update/Replace" event, I have to use IExecuteSystem.Execute in every frame loop to check some components value is updated.

And I find the source code of Group.cs, the "Group.OnEntityUpdated"(delegate variable) haven't been set at any place, that's meaning, a variable was declared, never been set and used, I think the autor @sschmid may have some reasons.

yuchting avatar Jan 24 '18 10:01 yuchting

Can you be specific about what you are trying to do? I think i might have misunderstood you.

Reactive systems will trigger on component update, just like when component is added for the first time.

FNGgames avatar Jan 24 '18 10:01 FNGgames

@FNGgames thanks for your patient, let me explain more by code and design requirement.

I have a list of behaviors data, each behavior data relates one component,

interface DoSomethingData DoFirstThingData -> DoFirstThingComponent -> DoFirstThingSystem DoSecondThingData -> DoSecondThingComponent -> DoSecondThingSystem WaitForSecondsData -> WaitForSecondsComponent -> WaitForSecondsSystem

Now, my config json text is a list of behaviors:

DoFirstThingData WaitForSecondsData DoSecondThingData WaitForSecondsData DoSecondThingData DoSecondThingData ....

obviously, I cannot AddDoSecondThing twice in one GameEntity in one frame loop, so I designed:

// in another system, I create a game entity for those behaviors
List<DoSomethingData> list;
GameEntity en = context.CreateEntity();
en.AddDoSomethingDataList(list);
en.AddDoSomethingStatus(Done); // Running, Done
en.AddDoSomethingIdx(0);
...

systems design:

class DoFirstThingSystem : IReactiveSystem{
    Execute(entities){
        entities.foreach(e) => {
            // do first thing 
            ...
            // done
            
            e.RemoveDoFirstThing();
            e.ReplaceDoSomethingStatus(Done);         
        }
    }
}
class DoSecondThingSystem : IReactiveSystem{
    // same routine for DoSecondThingSystem
}
class WaitForSecondsSystem : IExecuteSystem{
    entities = context.GetGroup(AllOf(WaitForSeconds));
    Execute(){
        entities.foreach(e) => {
            // wait a time by timer
            if(e.waitForSeconds.timer += Time.deltaTime > e.waitForSeconds.time){
                e.RemoveWaitForSeconds();
                e.ReplaceDoSomethingStatus(Done);  
            }                   
        }
    }
}
class CheckDoSomethingStatusSystem : IExecuteSystem{
    // I have to check and check again
    entities = context.GetGroup(AllOf(DoSomethingStatus));
    Execute(){
        entities.foreach(e){
            if(e.DoSomethingStatus.status == Done){
                e.DoSomethingIdx.idx += 1;
                if(e.DoSomethingIdx.idx >= e.DoSomthingList.list.Count){
                    // all behaviors are executed done
                    e.Destory();
                }else{
                    // add some thing by data
                    Utils.AddComponentForDoSomething(e,e.DoSomthingList.list[e.DoSomethingIdx.idx]);
                }               
            }
        }
    }
}

above code may have some bug, but you should what I need and what I am doing for this. Maybe you have another better way to realize my requirement.

So, If I have "Update/Replace" event trigger, I won't design a IExecuteSystem for CheckDoSomethingStatus, IReactiveSystem is better.

Cheers

yuchting avatar Jan 24 '18 13:01 yuchting

So if i understand you, you're looking to find away to replace that last execute system with a nice reactive one that doesn't need to check every frame?

At first glance I would say just make a reactive system that uses trigger: Matcher.DoSomethingStatus and filter: entity.hasDoSomethingStatus && entity.doSomthingStatus == Done. Every time the status is updated you check to see if the status is done, if not ignore it, if so, work on it.

Does that make any sense?

FNGgames avatar Jan 24 '18 14:01 FNGgames

Hey. I had this suggestion before (https://github.com/sschmid/Entitas-CSharp/issues/388). There are code entries you can use if you use source code version. It makes real split between added/updated/removed.

IDNoise avatar Jan 24 '18 19:01 IDNoise

@FNGgames I understand your meaning now, that's to say, I can write CheckDoSomethingStatusSystem like this:

class CheckDoSomethingStatusSystem : IReactiveSystem{
    
    Filter(){
        entity.hasDoSomethingStatus && entity.doSomethingStatus == Done;
    }
    
    GetTrigger(){
        return CreateCollector(GameMatcher.ActionStatus);
    }
    
    Execute(entities){
        entities.foreach(e){
            //if(e.DoSomethingStatus.status == Done){
                e.DoSomethingIdx.idx += 1;
                if(e.DoSomethingIdx.idx >= e.DoSomthingList.list.Count){
                    // all behaviors are executed done
                    e.Destory();
                }else{
                    // add some thing by data
                    Utils.AddComponentForDoSomething(e,e.DoSomthingList.list[e.DoSomethingIdx.idx]);
                }               
            //}
        }
    }
}

and then, if some systems call

e.ReplaceDoSomethingStatus(Done)
or
e.AddDoSomethingStatus(Done)

I will collect these entity and process by this system. Thanks for your explanation, so patient.

I just want to ask another un-resolved problem again, why in Entitas source code Group.cs, there is OnEntityUpdated to handle replace event only, but haven't realized it and remain a never used variables, it makes me confused.

And more, a method calling ReplaceDoSomethingStatus may raise two different events: Added if hasn't any Component before or Removed and Added if has one.

Maybe, if you design it one relates one: AddDoSomethingStatus just raises Added event RemoveDoSomethingStatus just raises Removed event ReplaceDoSomethingStatus just raises Replaced event if it has already one, if you haven't one, it raises Added event.

Is it a better design? Please don't mind, I just make a suggestion.

@IDNoise thanks for your help, do you mean, you open a issue before, but @sschmid haven't modified? And he suggests us do:

protected override Collector<GameEntity> GetTrigger(IContext<GameEntity> context) {
    return context.CreateCollector(
        GameMatcher.DoSomethingStatus.Removed(),
        GameMatcher.DoSomethingStatus.Added()
    );
}

rather than:

protected override Collector<GameEntity> GetTrigger(IContext<GameEntity> context) {
    return context.CreateCollector(GameMatcher.DoSomethingStatus.Updated());
}

yuchting avatar Jan 25 '18 01:01 yuchting

This has been discussed before as @IDNoise says - see issue #388.

FNGgames avatar Jan 25 '18 09:01 FNGgames

Hey, what's the status on this? We've actually ran into an issue with our code today where we didn't want our ReactiveSystem to trigger when something was Removed but only when it was Replaced. but since calling Replace will trigger both Added and Removed it made us do some ugly workarounds.

aviv-jellybtn avatar Jul 11 '18 15:07 aviv-jellybtn

Perhaps in the filter make sure you state that entity does not have the component so then when the system is triggered it’ll only execute when the entity does not have the component which would be similar to reacting on remove but with replace. Or just actually use the component generated remove method and then just use the add method. I’ve even ran into sometimes where although using replace is just faster to do using removed and then add was just better for my system filtering

RealCosmik avatar Jul 11 '18 18:07 RealCosmik