How to filter buffed entities based on scene id in client side?


#21

Well, I followed this and come with following implementation. @pspeed will appreciate if you comment on this.

Note, I am using Buff component instead of Parent component.

First my (actually it’s yours :slight_smile: ) Buff component :

/**
 * Associated with entities that apply some effect to another entity.
 *
 * @author Paul Speed
 */
public class Buff implements PersistentComponent {

    private EntityId target;
    private int type;
    private long startTime;
    private int state;

    public Buff() {
    }

    public Buff(EntityId target, int type) {
        this(target, type, 0);
    }

    public Buff(EntityId target, int type, long startTime) {
        this(target, type, startTime, 0);
    }

    public Buff(EntityId target, int type, int state) {
        this(target, type, 0, state);
    }

    public Buff(EntityId target, int type, long startTime, int state) {
        this.validateType(type);
        this.target = target;
        this.type = type;
        this.startTime = startTime;
        this.state = state;
    }

    public EntityId getTarget() {
        return target;
    }

    public int getType() {
        return type;
    }

    public long getStartTime() {
        return startTime;
    }

    public int getState() {
        return state;
    }

    public Buff adjustTarget(EntityId target){
         return new Buff(target, type, startTime, state);
    }
    
    public Buff adjustState(int state) {
        return new Buff(target, type, startTime, state);
    }

    @Override
    public String toString() {
        return "Buff[" + target + ", at:" + startTime + "]";
    }

    private void validateType(int type) {
        if (type == 0) {
            throw new RuntimeException("Unknow buff type!");
        }
    }
}

and BuffVisibility :

/**
 * Limits the client's visibility of any entity containing a Buff
 * to whatever is in the current scene client is attached to.
 * 
 * @author Ali-RS
 */
public class BuffVisibility implements ComponentVisibility {

    static Logger log = LoggerFactory.getLogger(BuffVisibility.class);

    private EntityData ed;
    private BufferedHashSet<EntityId> currentSceneBuffs;
    private BufferedHashSet<EntityId> lastSceneBuffs;

    /**
     * 
     * @param sceneBuffs containing all the buffed entities in current scene that client is attached.
     */
    public BuffVisibility(BufferedHashSet<EntityId> sceneBuffs) {
        this.currentSceneBuffs = sceneBuffs;
    }

    /**
     * Set new scene buffs if scene has changed.
     * 
     * @param sceneBuffs 
     */
    public void setCurrentSceneBuffs(BufferedHashSet<EntityId> sceneBuffs) {
        this.lastSceneBuffs = this.currentSceneBuffs;
        this.currentSceneBuffs = sceneBuffs;
    }

    @Override
    public Class<? extends EntityComponent> getComponentType() {
        return Buff.class;
    }

    @Override
    public void initialize(EntityData ed) {
        this.ed = ed;
    }

    @Override
    public <T extends EntityComponent> T getComponent(EntityId entityId, Class<T> type) {
        log.info("getComponent(" + entityId + ", " + type + ")");
        
        if (!currentSceneBuffs.contains(entityId)) {
            return null;
        }
        return ed.getComponent(entityId, type);
    }

    @Override
    public Set<EntityId> getEntityIds(ComponentFilter filter) {
        if (log.isTraceEnabled()) {
            log.trace("getEntityIds(" + filter + ")");
        }
        if (filter != null) {
            throw new UnsupportedOperationException("Filtering + buff visibility not yet supported");
        }
        return currentSceneBuffs.getSnapshot();
    }

    public boolean collectChanges(Queue<EntityChange> updates) {
        boolean changed = false;
//        if( log.isTraceEnabled() ) {
//            log.trace("active:" + active);
//            log.info("updates before:" + updates);
//        }

        // Remove buffs from previous scene
        if(lastSceneBuffs != null){
            updates.addAll(Collections2.transform(lastSceneBuffs.getSnapshot(), id -> new EntityChange(id, Buff.class)));
            lastSceneBuffs = null;
        }

        // Remove any Buff updates that don't belong to the client scene
        for (Iterator<EntityChange> it = updates.iterator(); it.hasNext();) {
            EntityChange change = it.next();
            if (change.getComponentType() == Buff.class
                    && !currentSceneBuffs.contains(change.getEntityId())) {
                if (log.isTraceEnabled()) {
                    log.trace("removing irrelevant change:" + change);
                }
                it.remove();
            }
        }
        return changed;
    }
}

and finally BuffSystem :

/**
 * Keep track of the buff IDs for each scene.
 * 
 * @author Ali-RS
 */
public class BuffSystem extends AbstractGameSystem {

    static Logger log = LoggerFactory.getLogger(BuffSystem.class);

    private EntityData ed;
    private EntitySet buffSet;

    // Keep track of buffs by sceneId 
    private final Map<EntityId, BufferedHashSet<EntityId>> buffIndex = new HashMap();
    // Keep track of sceneId by buffId
    private final Map<EntityId, EntityId> sceneIndex = new HashMap<>();
    private final Queue<EntityId> pendingCommit = new ArrayDeque<>();

    public BufferedHashSet<EntityId> getBuffs(EntityId sceneId) {
        synchronized (buffIndex) {
            return buffIndex.get(sceneId);
        }
    }

    @Override
    protected void initialize() {
        ed = getSystem(EntityData.class, true);
        buffSet = ed.getEntities(Buff.class);
    }

    @Override
    protected void terminate() {
        buffSet.release();
        buffIndex.clear();
    }

    @Override
    public void update(SimTime time) {
 
        if (buffSet.applyChanges()) {
            for (Entity entity : buffSet.getAddedEntities()) {
                addBuff(entity);
            }

            for (Entity entity : buffSet.getRemovedEntities()) {
                removeBuff(entity);
            }
        }

        applyPendingCommit();
    }

    private void addBuff(Entity entity) {
        Buff buff = entity.get(Buff.class);
        switch (buff.getType()) {
            case BuffTypes.BODY:
                EntityId sceneId = buff.getTarget();
                getBuffSet(sceneId).add(entity.getId());
                sceneIndex.put(entity.getId(), sceneId);
                addToCommit(sceneId);
                break;
            case BuffTypes.MOBILITY:
            case BuffTypes.ACTION:
                EntityId bodyId = buff.getTarget();
                sceneId = buffSet.getEntity(bodyId).get(Buff.class).getTarget();
                getBuffSet(sceneId).add(entity.getId());
                sceneIndex.put(entity.getId(), sceneId);
                addToCommit(sceneId);
                break;
        }
    }

    private void removeBuff(Entity entity) {
        EntityId sceneId = sceneIndex.remove(entity.getId());
        buffIndex.get(sceneId).remove(entity.getId());
        addToCommit(sceneId);
    }

    private BufferedHashSet<EntityId> getBuffSet(EntityId sceneId) {
        if (!buffIndex.containsKey(sceneId)) {
            buffIndex.put(sceneId, new BufferedHashSet<>());
        }
        return buffIndex.get(sceneId);
    }

    private void addToCommit(EntityId sceneId) {
        if (!pendingCommit.contains(sceneId)) {
            pendingCommit.add(sceneId);
        }
    }

    private void applyPendingCommit() {
        while (!pendingCommit.isEmpty()) {
            EntityId sceneId = pendingCommit.poll();
            buffIndex.get(sceneId).commit();
        }
    }
}

#22

I guess an entity cannot ever change scenes?

I still don’t really understand the point of the scene ID or why that’s a property of Buff… but that’s your design not mine. :slight_smile:


#23

See… from an ES design perspective, this is the kind of thing that totally baffles me. What do these totally different aspects have to do with one another. Why is “BODY” a buff type and “ACTION” a buff type. It feels like something is missing in the design when these two basically unrelated things find themselves siblings in some switch/case block.


#24

Ah, I think you got me wrong.
Yes an entity can not change scene except the client avatar entity. And that’s my point here :

 /**
     * Set new scene buffs if scene has changed.
     * 
     * @param sceneBuffs 
     */
    public void setCurrentSceneBuffs(BufferedHashSet<EntityId> sceneBuffs) {
        this.lastSceneBuffs = currentSceneBuffs;
        this.currentSceneBuffs = sceneBuffs;
    }

By scene I mean level. When client wins level 1 (scene 1) his avatar is going to detach from that scene and will be attached to scene 2 (level 2) and any other entity in that scene is going to be decayed.

Now I think this line is completely unnecessary and wrong, because these entities will be decayed anyway.

// Remove buffs from previous scene
        if(lastSceneBuffs != null){
            updates.addAll(Collections2.transform(lastSceneBuffs.getSnapshot(), id -> new EntityChange(id, Buff.class)));
            lastSceneBuffs = null;
        }

Am I clear at this point ? Anything seems wrong at this point ?


#25

I think you will someday regret Buff in a very large way. Sometimes it points to a scene, sometimes it points to a body. Very strange.

Anyway, to me, scene is an attribute of Position… whatever you are using to represent position needs a scene ID. Treating things as a buff on scene feels really wrong.

I think your willingness to corrupt concepts for convenience will get you into trouble. But it could be that I’m reading the code wrong… but these kinds of switch/cases on a component type are super-stinky design smells.


#26

Foreach is more performant because it does an internal iteration.

That just means it does this:

int i;
for (i = 0; i < someLength; i++)

It “caches” the iterator index. It’s a micro optimization. A bit like doing ++i instead of i++ in that it does work but I wouldn’t lose sleep. You see it a LOT in JavaScript, though. Probably in a final pass of the code.

I use lambdas and linq type syntax when it makes sense. I find them to be more performant in many cases but not all. It really depends on the case and it can make a noticeable difference. The only real way to know is to follow your hunch and give it a try.

Take the removeIf method for example. I’m not certain how it works internally but I’m more certain than not that it will be better if not exactly the same as any implementation I could put together. So just use it.


#27

Okay, I want to find out what is wrong with my design and correct it. I need your help. :slightly_smiling_face:

Here is the big picture, I hope it is clear enough :

Buff can have lots of type. For this reason I have a field type in Buff component.

And my BuffTypes are :

public class BuffTypes {
    
    public static final int BODY                  = 1;
    public static final int COLLISION_SHAPE       = 2; 
    public static final int ITEM                  = 3; 
    public static final int STAT                  = 4;
    public static final int REQUEST               = 5;
    public static final int SCENE                 = 6;
    public static final int QUEST                 = 7;
    public static final int PARTY                 = 8;
    public static final int ACCOUNT               = 9;
    public static final int ACTION                = 10;
    public static final int MOBILITY              = 11;
}

Client Account entity must buff to a Party entity.
Scene entity must buff to a Party entity.
Body entity must buff to a Scene entity.
A mob body can have multiple Mobility component buffed to it.
A mob body can have multiple CharacterAction component buffed to it.
A body with Attribute component can have multiple Stat buffed to it.

Do you think using a general Buff component here is wrong ?
Then what is your suggestion ?
Should I use different component for each different buff type ?

I wanted to use BuffVisibility to filter out any buff entity that not belong to the client scene. This can be a Body which is buffed to a Scene, Mobility or CharacterAction or Stat which is buffed to a Body.

So that in client side for example in MobAnimationState I will be sure that MobilityContainer and ActionContainer will not see entities that they are not supposed to see.

 @SuppressWarnings("unchecked")
    private class MobilityContainer extends EntityContainer<BuffedComponent<Mobility>> {

        public MobilityContainer(EntityData ed) {
            super(ed, Buff.class, Mobility.class);
        }

        @Override
        protected BuffedComponent<Mobility> addObject(Entity e) {
            Buff p = e.get(Buff.class);
            Mobility m = e.get(Mobility.class);
            addMobility(p.getTarget(), m);
            return new BuffedComponent<>(p.getTarget(), m);
        }

        @Override
        protected void updateObject(BuffedComponent<Mobility> object, Entity e) {
        }

        @Override
        protected void removeObject(BuffedComponent<Mobility> object, Entity e) {
            removeMobility(object.parentId, object.value);
        }
    }

    @SuppressWarnings("unchecked")
    private class ActionContainer extends EntityContainer<BuffedComponent<CharacterAction>> {

        public ActionContainer(EntityData ed) {
            super(ed, Buff.class, CharacterAction.class);
        }


        @Override
        protected BuffedComponent<CharacterAction> addObject(Entity e) {
            Buff p = e.get(Buff.class);
            CharacterAction a = e.get(CharacterAction.class);
            addAction(p.getTarget(), a);
            return new BuffedComponent<>(p.getTarget(), a);
        }

        @Override
        protected void updateObject(BuffedComponent<CharacterAction> object, Entity e) {
        }

        @Override
        protected void removeObject(BuffedComponent<CharacterAction> object, Entity e) {
            removeAction(object.parentId, object.value);
        }
    }

I have no choice, Bullet physics does not support the idea of scenes internally so I should use different PhysicsSpace for each scene.

In BulletSystem I create one PhysicsSpace for each Scene and add all bodies buffed to that Scene to it’s own PhysicsSpace.


#28

@pspeed what exactly seems wrong and broken in my design and how should I fix it ?
I appreciate your help


#29

I am using buff type so I can know that if it is a Body then this implies that it’s target is a Scene.
If it is a Mobility or CharacterAction then it’s target is a body so I can get that body and find the scene id from body.

I am doing this to group all buff entities by scene ID in BuffSystem.

// Keep track of buffs by sceneId 
    private final Map<EntityId, BufferedHashSet<EntityId>> buffIndex = new HashMap();

so that in BuffVisibility the collectChanges method can do it’s job correctly.

 public boolean collectChanges(Queue<EntityChange> updates) {
        boolean changed = false;
//        if( log.isTraceEnabled() ) {
//            log.trace("active:" + active);
//            log.info("updates before:" + updates);
//        }

        // Remove any Buff updates that don't belong to the client scene
        for (Iterator<EntityChange> it = updates.iterator(); it.hasNext();) {
            EntityChange change = it.next();
            if (change.getComponentType() == Buff.class
                    && !currentSceneBuffs.contains(change.getEntityId())) {
                if (log.isTraceEnabled()) {
                    log.trace("removing irrelevant change:" + change);
                }
                it.remove();
            }
        }
        return changed;
    }

#30

ehm, I think there is another way to do that. I can just set scene id as a field in Buff component :slightly_smiling_face:

But there is a big problem with this approach. Why should a Mobility entity or Stat entity needs to know in what scene it is ?
Suppose character avatar, it can have lots of Stat buffed to it, if we save scene id in Buff component then when character avatar switches to other scene then we should update scene ids in all of it’s Stats.

By Stat I mean this

public class Stat implements PersistentComponent {

    private int type;
    private int value;
    private int maxValue;

    public Stat() {
    }

    public Stat(int type, int value, int maxValue) {
        this.type = type;
        this.value = value;
        this.maxValue = maxValue;
    }

    public static Stat create(String type, int value, int maxValue, EntityData ed){
        Stat stat = new Stat();
        stat.type = ed.getStrings().getStringId(type, true);
        stat.value = value;
        stat.maxValue = maxValue;
        return stat;
    }
    public int getType() {
        return type;
    }
    
    public String getTypeName(EntityData ed){
        return ed.getStrings().getString(type);
    }

    public int getValue() {
        return value;
    }

    public int getMaxValue() {
        return maxValue;
    }

    public Stat newAdjusted(int delta) {
        return new Stat(type, value + delta, maxValue);
    }
}

#31

I don’t understand why you have Buff(includes all of the things) intead of Parent, Mobility… Parent, SomethingElse… Parent, SomethingElseStill. (Edit: or Target, Mobility… Target, SomethingElse… etc. if you like)

Essentially, by building all of these types into one component, you are making every system depend on each other. Adding one new constant technically affects ALL systems. You might as well just have a BaseGameObject class and go back to plain-old-OOP because you are essentially destroying one of the big benefits of ES.

Also, your explanation for why the Position component (that you control as it’s not at all a part of Bullet) cannot have a SceneId on it. Somehow positions are being applied to the entities can could include the scene ID.


#32

Actually at first they where separate, (TerrainBlockBuff, BridgeBlockBuff, SceneBuff, ActionBuff, StatBuff, ItemBuff, … ) but when I find all are just a pointer to target I merged them into one component called Buff. I did not know this would break ES design. Then I will go back and use a separate component for each as you suggested.

@pspeed then is it OK if I implement a ComponentVisibility for each of them ?


#33

Ugh… I think we are still not communicating well.

What would TerrainBlockBuff have in it? What would BridgeBlockBuff have in it? Etc…

(Buff is still a really weird word to use here, by the way… these don’t seem like temporary enhancements to me but it’s your game and you are the only one who really has to read it in the end. I just have to relearn that “buff” doesn’t mean buff almost every time I wade into this thread.)


#34

I am giving it a second try now. I am rethinking the design. I will come up with new approach and proper naming. :slightly_smiling_face:

I will let you know about it soon. Thanks so much for all the helps.


#35

Add SceneId to Position, use RMI to tell client which SceneId to filter on (used to pre-loading I guess as well) and also which scene to attach to? Not sure if this has already been discussed :stuck_out_tongue:


#36

So, previously I was using a Buff component in all situations to point to a target entity but after taking a look at MonkeyTrap example again, I want to follow a new approach.

  • Buff component :
    Associated with entities that apply some effect to another entity. I will use it with Stat components. (for example Health, XP, Strength, Mana, AttackRange, … stats)

    public class Stat implements PersistentComponent {
    
     private int type;
     private int value;
     private int maxValue;
    
     public Stat() {
     }
    
     public Stat(int type, int value, int maxValue) {
         this.type = type;
         this.value = value;
         this.maxValue = maxValue;
     }
    
     public static Stat create(String type, int value, int maxValue, EntityData ed){
         Stat stat = new Stat();
         stat.type = ed.getStrings().getStringId(type, true);
         stat.value = value;
         stat.maxValue = maxValue;
         return stat;
     }
     public int getType() {
         return type;
     }
     
     public String getTypeName(EntityData ed){
         return ed.getStrings().getString(type);
     }
    
     public int getValue() {
         return value;
     }
    
     public int getMaxValue() {
         return maxValue;
     }
    
     public Stat newAdjusted(int delta) {
         return new Stat(type, value + delta, maxValue);
     }
    }
    
  • ItemBuff component :
    Associated with entities that when acquired, applies some effect to another entity. This will be used with items (weapons, attacks, spells, potions), when acquired they are going to apply their stats to player.

  • Parent component :
    I do not know when should I use it exactly, @pspeed uses it for Mobility and CharacterAction components so I am supposing it should be used when an entity is targetting mob body ?

Okay, now I am using a property “sceneId” on SpawnPosition and BodyPosition components to point to scene id instead of buffing it to scene.

Previously I was buffing client Account entity to Party entity it is joined, now I am going to use a property “partyId” inside Account component.

@pspeed looking forward to hear your idea about new approach. :slightly_smiling_face:


#37

I’m thinking you only need it once. After spawning, the client knows the sceneid and doesn’t need the information on every BodyPosition. If the client should connect to a different scene, it will receieve a new SpawnPosition with a new SceneId?


#38

Now it’s starting to ‘dawn’ on me how you use this word :slight_smile:


#39

Hmm, Yes, you are right. I think no need to add it again to BodyPosition.


#40

Just easier to filter BodyPosition if they have it but technically it’s redundant information, I guess.

…unless bodies can move from scene to scene after spawning.