pspeed
February 17, 2019, 8:20pm
21
Yeah, this line:
set.release(); } public void getStringInfo( HostedConnection source, StringIdMessage msg ) { if( msg.getId() != null ) { source.send(new StringIdMessage(msg.getRequestId(), ed.getStrings().getString(msg.getId()))); } else if( msg.getString() != null ) { source.send(new StringIdMessage(msg.getRequestId(), ed.getStrings().getStringId(msg.getString(), false))); } else { throw new RuntimeException("Bad StringIdMessage:" + msg); } } protected void sendAndClear( int setId, List<ComponentData> buffer ) { conn.send(settings.getChannel(), new EntityDataMessage(setId, buffer)); buffer.clear(); } protected void sendAndClear( List<EntityChange> buffer ) {
Is invoking this constructor:
// Have to make 'id' an Object field because making // it an Integer field confuses the Serializer and it // won't send nulls... but NPE instead. private Object id; private String value; public StringIdMessage() { } public StringIdMessage( int requestId, int id ) { this.requestId = requestId; this.id = id; } public StringIdMessage( int requestId, String value ) { this.requestId = requestId; this.value = value; } public int getRequestId() {
…which won’t take null because it wants to unbox the object.
I think it’s better to let the -1 through and then make sure the client detects it and doesn’t cache the value.
The problem is that I have no way to test this code because I tend to construct my apps in such a way as to avoid it.
1 Like
Ali_RS
February 18, 2019, 6:11am
22
@pspeed can you take a look at this please ?
https://github.com/jMonkeyEngine-Contributions/zay-es/pull/22
Now the result != null
seems would never happen, should I get rid of it ?
// Check the result again because between unlock // and lock something else might have requested the same // string. String result = stringIndex.get(id); if( result != null ) { return result; } // Else ask remote result = remote.getString(id); if( result != null && result > 0 ) { idIndex.put(result, id); stringIndex.put(id, result); } return result; } finally { indexLock.writeLock().unlock(); } } }
pspeed
February 18, 2019, 6:22am
23
Umm… isn’t that for getting the string and not the id?
result should be a string in this case and might be null.
I thought your bug was related to this method:
private Map<String, Integer> idIndex = new ConcurrentHashMap<String, Integer>(); private Map<Integer, String> stringIndex = new ConcurrentHashMap<Integer, String>(); private ReadWriteLock indexLock = new ReentrantReadWriteLock(); protected RemoteStringIndex( RemoteEntityData remote ) { this.remote = remote; } @Override public int getStringId( String s, boolean add ) { if( add == true ) { throw new UnsupportedOperationException("Clients cannot create new string mappings."); } indexLock.readLock().lock(); try { Integer result = idIndex.get(s); // Note: misses are not cached... we'd have no way to detect // if they were filled in later. if( result != null ) { // We're done.
…where you are trying to look up the integer ID for a string that hasn’t been registered yet.
Ali_RS
February 18, 2019, 6:25am
24
Oh, what a mistake , I did it in wrong place. Sorry gonna fix it.
Ali_RS
February 18, 2019, 6:48am
25
pspeed
February 18, 2019, 8:33am
28
Merged and release note added.
1 Like
Ali_RS
February 18, 2019, 8:53am
29
Are you going to release Zay-ES-Net v1.4.1 as hot fix ? Or you want to keep it until the next release iteration ?
pspeed
February 18, 2019, 9:31am
30
I’m just going to wait. So far this seems to have not affected many people.
1 Like
pspeed
February 18, 2019, 9:33am
31
Also, I recently started added automated tests to SimMath… one of the places where they really do make sense.
I’ve been trying to think if there is a way to test stuff like this in an automated way. Network stuff is notoriously difficult to test in ‘unit test’ form factor but it’s on my mind. I’d like to think about it a little more before the next release.
Zay-ES core is another module that could really use some unit testing… like SimMath, it’s ripe for it because behavior is well defined and consistent. Zay-ES-Net is a little harder because there is no easy loopback mechanism and few places to inject one.
1 Like