Zay-es Bug in multiplayer? Entities are in the wrong sets

I ran into this as well yesterday. If needed I can share the details.

I got this bug now, and I canā€™t seem to work around it by fixing the order of the components either.

Iā€™ve checked the evaluate results on the filter, and thereā€™s no ā€˜wrongā€™ comparisons there there. It filters on the right field and compares the right values using com.google.common.base.Objects.equal(rightValue, wrongValue)

From this I guess itā€™s added to a set (in this case itā€™s a EntityContainer with the filter that gets the wrong entity) even though the filter eval function returns false.

You guys could always set a breakpoint in the debugger and see whatā€™s happening.

ā€¦thatā€™s essentially what I will do when I have time to look at the test case.

Sorry, to bumb this thread againā€¦.
I assume, that the source of this bug has still not be found yet, right?

Correct. Itā€™s possible that I will hit this myself soon.

ā€¦or you guys could set a breakpoint in the debugger and see whatā€™s happening as mentioned above.

Alright, event though I am quite busy at the moment (probably like everybody else here), I nevertheless will try to investigate this issue next week or so. Wish me the best of luck :slightly_smiling_face:

2 Likes

Hello @pspeed,

I got some time to get a look at thisā€¦ I debugged the test case and I think I found the bug in the client-server communication. I will now try to describe how I come to my conclusion.

It seems that the client does not do any filtering of the components at all because it assumes that the server does already filter the components for its EntitySets which does by the way work perfectly so far.

But now imagine the test-case from above where there are two EntitySets which have the same components but each EntitySet filters for a different value of that component (see FieldFilter with x==0 and x==1).

The server will send the changes to the client via ComponentChangeMessages. This is done in this part of the HostedEntityData class.

   for( EntityChange change : frameChanges ) {
            
            Long last = tracker.getAndExpire(change.getEntityId(), change.getComponentType(),
                                             frame);
            
            // Three cases:
            // a) last and frame are the same and we need to send the change
            // b) last and frame are different... we need to send the change
            //    but we also need to sweep it (which we just did)
            // c) last is null meaning we don't watch this combo... skip it.
            if( last == null ) {
                // Skip it as we don't track this particular ID + type combo
                continue;
            }
            
            // Buffer the updates            
            changeList.add(change);
            if( changeList.size() > changeMax ) {
                sendAndClear(changeList);
            } 
        }

Alright, in the next step, the client will receive those update messages in the RemoteEntityData class and will add the messages to the changes list.

public void componentChange( ComponentChangeMessage msg ) {
    if( log.isTraceEnabled() ) {
        log.trace("componentChange(" + msg + ")");
    } 
    for( EntityChange c : msg.getData() ) {
        entityChange(c);
    }
}

This will add the change to each EntitySet of the client as follows.

protected void entityChange( EntityChange change ) {

    for( RemoteEntitySet set : activeSets.values() ) {
        set.entityChange(change);
    }
    
    for( RemoteWatchedEntity e : watchedEntities.values() ) {
        e.addChange(change);
    }       
}

All fine so far, but when the user calls applyChanges() then methos like buildTransactionChanges(), completeEntity() etc. are called. If you have a look at the completeEntity() method of the RemoteEntitySet you see that there is no filtering done.

   @Override
    protected boolean completeEntity( DefaultEntity e ) {
 
        // In a remote situation, the server is (at least now) 
        // always sending us what we need.  If the entity was
        // newly added to this set then it sent us the full
        // entity.  Else we've gotten every change we needed
        // to keep it relevant.
        //
        // So all we need to do is check it for completion and
        // not bother retrieving the values.
 
        EntityComponent[] array = e.getComponents();
        for( int i = 0; i < array.length; i++ ) {
            if( array[i] == null || array[i] == REMOVED_COMPONENT ) {
                if( log.isTraceEnabled() ) {
                    // Logging this because if the assumptions above ever
                    // prove false then it would be nice to have some trace
                    // logging to remind me of the optimization.  (It's a big
                    // huge optimization so it's definitely worth having.)
                    log.trace("Entity is missing type " + getTypes()[i] + " so is not complete for this set.");
                }
                return false;                     
            } else {
                // Nothing to see here
            }
        }        
        
        return true;
    }

In a non-networked situation the filtering process is done in this method but not in a remote situation because here the client assumes that the server will only send the components which are relevant (seems to include filtered components).

Donā€™t get me wrong, the filtering does work so far, but for that special case with two EntitySets filtering for the same component type for two different values will result in this bug.

In the end, the completeEntity() method will only check if there are all component types for that entity in the set (not filtering for component values or so) which results in the adding of that entity to the addedEntities list/set.

We could probably fix this issue by including the filter in one of the methods (probably in the completeEntity() method). Maybe I could propse a fix.

Okay, well, that would be it then. Hopefully I could help you a little bit with that.

Best regards
Domenic


EDIT:

Created a pull request which fixes this bug:

4 Likes

I will take a look this weekend. Thanks for the legwork so far.

1 Like

Hi,

did you have a look at this? What do you think about the fix?

Best regards
Domenic

Not yetā€¦ sorry. I promise to look soon. I plan to take some time off work soon and if that happens I hope to catch up on a bunch of these backlog items.

Anything new?

Sorryā€¦ Iā€™m a horrible person and have found myself easily distracted by a hundred different things. I do still have this window open, thoughā€¦ like, literally, I keep this browser window open to remind me to do something about it.

ā€¦I may need a new strategy. :slight_smile:

I keep hoping that my ā€œlittleā€ game project will reach the point where it needs this fix and I can kill two birds with one stone. But that may not happen.

No, you are not a horrible person. I know that feeling :wink:

Well, what if you never reach this point? Will the bug then never be fixed? I mean, does it really take that long to just take a look at the problem and the fix, then discuss a little about it and eventually push a fix. At least there was someone who did the legwork for you already :wink:

It just means that Iā€™d have to test it myself. The fix still happens.

ā€¦itā€™s just easier if Iā€™m already hitting the problem, apply the patchā€¦ and it works. Then I can move on with my life. :slight_smile:

Sorry, i am not a native English speaker, but what do you mean with ā€œThe fix still happensā€ ?

Alright :wink:

Fortunately, I donā€™t need multiplayer for my next game :wink: However, I still will use Zay-Es because it is just awesome! Well done on that mate :slight_smile:

I mean, I still apply the fix somehow even if I donā€™t have the problem myself. You asked the questions. I was answering.

The bug will be fixed either way.

Finally looked at this and applied the patch. Sorry for dragging this out.

Iā€™m especially paranoid about these sorts of changes in core code for a module that has been super stable up until this point. Thanks sooooo much for finding and fixing this issue.

4 Likes