Skip to content

Commit f1bd2ed

Browse files
irrldSupremeMortal
irrld
authored and
SupremeMortal
committed
Fix EntityDataMap serialization to only write present flag groups
1 parent c0fc2e8 commit f1bd2ed

File tree

9 files changed

+365
-124
lines changed

9 files changed

+365
-124
lines changed

‎adventure/src/test/java/org/cloudburstmc/protocol/bedrock/test/TextSerializationTest.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,15 @@
2020
import org.cloudburstmc.protocol.bedrock.data.Ability;
2121
import org.cloudburstmc.protocol.bedrock.data.AbilityLayer;
2222
import org.cloudburstmc.protocol.bedrock.data.GameType;
23+
import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataMap;
2324
import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataTypes;
2425
import org.cloudburstmc.protocol.bedrock.data.entity.EntityFlag;
2526
import org.cloudburstmc.protocol.bedrock.data.inventory.ItemData;
2627
import org.cloudburstmc.protocol.bedrock.packet.AddPlayerPacket;
2728
import org.cloudburstmc.protocol.bedrock.packet.TextPacket;
2829
import org.junit.jupiter.api.Test;
2930

30-
import java.util.Arrays;
31-
import java.util.EnumSet;
32-
import java.util.List;
33-
import java.util.UUID;
31+
import java.util.*;
3432
import java.util.stream.Collectors;
3533
import java.util.stream.StreamSupport;
3634

@@ -192,7 +190,7 @@ public void testComplexMetadataSerialization() {
192190
packet.getMetadata().put(EntityDataTypes.HEIGHT, 0.6f);
193191
packet.getMetadata().put(EntityDataTypes.NAMETAG_ALWAYS_SHOW, (byte) 0);
194192

195-
packet.getMetadata().putFlags(EnumSet.of(EntityFlag.SILENT));
193+
packet.getMetadata().putFlags(EntityDataMap.flagsOf(EntityFlag.SILENT));
196194

197195
ByteBuf buf = Unpooled.buffer();
198196
PLAYER_SERIALIZER.serialize(buf, CODEC_HELPER, packet);

‎bedrock-codec/build.gradle.kts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ dependencies {
77
api(libs.jose4j)
88
api(libs.nbt)
99
implementation(libs.jackson.annotations)
10+
11+
// Tests
12+
testImplementation(libs.junit)
1013
}
1114

1215
tasks.jar {

‎bedrock-codec/src/main/java/org/cloudburstmc/protocol/bedrock/codec/v291/BedrockCodecHelper_v291.java‎

Lines changed: 58 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
import org.cloudburstmc.protocol.bedrock.data.command.CommandOriginData;
1818
import org.cloudburstmc.protocol.bedrock.data.command.CommandOriginType;
1919
import org.cloudburstmc.protocol.bedrock.data.definitions.ItemDefinition;
20-
import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataFormat;
21-
import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataMap;
22-
import org.cloudburstmc.protocol.bedrock.data.entity.EntityDataType;
23-
import org.cloudburstmc.protocol.bedrock.data.entity.EntityLinkData;
20+
import org.cloudburstmc.protocol.bedrock.data.entity.*;
2421
import org.cloudburstmc.protocol.bedrock.data.inventory.ItemData;
2522
import org.cloudburstmc.protocol.bedrock.transformer.EntityDataTransformer;
2623
import org.cloudburstmc.protocol.common.util.TriConsumer;
@@ -236,7 +233,7 @@ public void writeGameRuleInStartGame(ByteBuf buffer, GameRuleData<?> gameRule) {
236233

237234
@Override
238235
public void readEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
239-
checkNotNull(entityDataMap, "entityDataDictionary");
236+
checkNotNull(entityDataMap, "entityDataMap");
240237

241238
int length = VarInts.readUnsignedInt(buffer);
242239
checkArgument(this.encodingSettings.maxListSize() <= 0 || length <= this.encodingSettings.maxListSize(), "Entity data size is too big: %s", length);
@@ -285,7 +282,7 @@ public void readEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
285282
EntityDataTransformer<Object, ?> transformer = (EntityDataTransformer<Object, ?>) definition.getTransformer();
286283
Object transformedValue = transformer.deserialize(this, entityDataMap, value);
287284
if (transformedValue != null) {
288-
entityDataMap.put(definition.getType(), transformer.deserialize(this, entityDataMap, value));
285+
entityDataMap.put(definition.getType(), transformedValue);
289286
}
290287
}
291288
} else {
@@ -297,60 +294,75 @@ public void readEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
297294
@SuppressWarnings("unchecked")
298295
@Override
299296
public void writeEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
300-
checkNotNull(entityDataMap, "entityDataDictionary");
297+
checkNotNull(entityDataMap, "entityDataMap");
301298

302-
VarInts.writeUnsignedInt(buffer, entityDataMap.size());
299+
// Collect serialized entries first
300+
List<Map.Entry<EntityDataTypeMap.Definition<?>, Object>> serializedEntries = new LinkedList<>();
303301

304302
for (Map.Entry<EntityDataType<?>, Object> entry : entityDataMap.entrySet()) {
305303
EntityDataTypeMap.Definition<?> definition = this.entityData.fromType(entry.getKey());
306304

307-
VarInts.writeUnsignedInt(buffer, definition.getId());
308-
VarInts.writeUnsignedInt(buffer, definition.getFormat().ordinal());
309-
310305
try {
311306
Object value = ((EntityDataTransformer<?, Object>) definition.getTransformer())
312307
.serialize(this, entityDataMap, entry.getValue());
313308

314-
switch (definition.getFormat()) {
315-
case BYTE:
316-
buffer.writeByte((byte) value);
317-
break;
318-
case SHORT:
319-
buffer.writeShortLE((short) value);
320-
break;
321-
case INT:
322-
VarInts.writeInt(buffer, (int) value);
323-
break;
324-
case FLOAT:
325-
buffer.writeFloatLE((float) value);
326-
break;
327-
case STRING:
328-
writeString(buffer, (String) value);
329-
break;
330-
case NBT:
331-
this.writeItem(buffer, ItemData.builder()
332-
.definition(ItemDefinition.LEGACY_FIREWORK)
333-
.damage(0)
334-
.count(1)
335-
.tag((NbtMap) value)
336-
.build());
337-
break;
338-
case VECTOR3I:
339-
writeVector3i(buffer, (Vector3i) value);
340-
break;
341-
case LONG:
342-
VarInts.writeLong(buffer, (long) value);
343-
break;
344-
case VECTOR3F:
345-
writeVector3f(buffer, (Vector3f) value);
346-
break;
347-
default:
348-
throw new UnsupportedOperationException("Unknown entity data type " + definition.getFormat());
309+
// Skip if transformer returns null (indicating this entry shouldn't be serialized)
310+
if (value == null) {
311+
continue;
349312
}
313+
314+
serializedEntries.add(new AbstractMap.SimpleEntry<>(definition, value));
350315
} catch (Exception e) {
351316
throw new IllegalArgumentException("Failed to encode EntityData " + definition.getId() + " of " + definition.getType().getTypeName(), e);
352317
}
353318
}
319+
320+
VarInts.writeUnsignedInt(buffer, serializedEntries.size());
321+
322+
for (Map.Entry<EntityDataTypeMap.Definition<?>, Object> entry : serializedEntries) {
323+
EntityDataTypeMap.Definition<?> definition = entry.getKey();
324+
Object value = entry.getValue();
325+
326+
VarInts.writeUnsignedInt(buffer, definition.getId());
327+
VarInts.writeUnsignedInt(buffer, definition.getFormat().ordinal());
328+
329+
switch (definition.getFormat()) {
330+
case BYTE:
331+
buffer.writeByte((byte) value);
332+
break;
333+
case SHORT:
334+
buffer.writeShortLE((short) value);
335+
break;
336+
case INT:
337+
VarInts.writeInt(buffer, (int) value);
338+
break;
339+
case FLOAT:
340+
buffer.writeFloatLE((float) value);
341+
break;
342+
case STRING:
343+
writeString(buffer, (String) value);
344+
break;
345+
case NBT:
346+
this.writeItem(buffer, ItemData.builder()
347+
.definition(ItemDefinition.LEGACY_FIREWORK)
348+
.damage(0)
349+
.count(1)
350+
.tag((NbtMap) value)
351+
.build());
352+
break;
353+
case VECTOR3I:
354+
writeVector3i(buffer, (Vector3i) value);
355+
break;
356+
case LONG:
357+
VarInts.writeLong(buffer, (long) value);
358+
break;
359+
case VECTOR3F:
360+
writeVector3f(buffer, (Vector3f) value);
361+
break;
362+
default:
363+
throw new UnsupportedOperationException("Unknown entity data type " + definition.getFormat());
364+
}
365+
}
354366
}
355367

356368
@Override

‎bedrock-codec/src/main/java/org/cloudburstmc/protocol/bedrock/codec/v361/BedrockCodecHelper_v361.java‎

Lines changed: 52 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import org.cloudburstmc.protocol.common.util.TypeMap;
1717
import org.cloudburstmc.protocol.common.util.VarInts;
1818

19-
import java.util.Map;
19+
import java.util.*;
2020

2121
import static org.cloudburstmc.protocol.common.util.Preconditions.checkArgument;
2222
import static org.cloudburstmc.protocol.common.util.Preconditions.checkNotNull;
@@ -29,7 +29,7 @@ public BedrockCodecHelper_v361(EntityDataTypeMap entityData, TypeMap<Class<?>> g
2929

3030
@Override
3131
public void readEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
32-
checkNotNull(entityDataMap, "entityDataDictionary");
32+
checkNotNull(entityDataMap, "entityDataMap");
3333

3434
int length = VarInts.readUnsignedInt(buffer);
3535
checkArgument(this.encodingSettings.maxListSize() <= 0 || length <= this.encodingSettings.maxListSize(), "Entity data size is too big: %s", length);
@@ -79,7 +79,7 @@ public void readEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
7979
EntityDataTransformer<Object, ?> transformer = (EntityDataTransformer<Object, ?>) definition.getTransformer();
8080
Object transformedValue = transformer.deserialize(this, entityDataMap, value);
8181
if (transformedValue != null) {
82-
entityDataMap.put(definition.getType(), transformer.deserialize(this, entityDataMap, value));
82+
entityDataMap.put(definition.getType(), transformedValue);
8383
}
8484
}
8585
} else {
@@ -91,55 +91,69 @@ public void readEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
9191
@SuppressWarnings("unchecked")
9292
@Override
9393
public void writeEntityData(ByteBuf buffer, EntityDataMap entityDataMap) {
94-
checkNotNull(entityDataMap, "entityDataDictionary");
94+
checkNotNull(entityDataMap, "entityDataMap");
9595

96-
VarInts.writeUnsignedInt(buffer, entityDataMap.size());
96+
// Collect serialized entries first
97+
List<Map.Entry<EntityDataTypeMap.Definition<?>, Object>> serializedEntries = new LinkedList<>();
9798

9899
for (Map.Entry<EntityDataType<?>, Object> entry : entityDataMap.entrySet()) {
99100
EntityDataTypeMap.Definition<?> definition = this.entityData.fromType(entry.getKey());
100101

101-
VarInts.writeUnsignedInt(buffer, definition.getId());
102-
VarInts.writeUnsignedInt(buffer, definition.getFormat().ordinal());
103-
104102
try {
105103
Object value = ((EntityDataTransformer<?, Object>) definition.getTransformer())
106104
.serialize(this, entityDataMap, entry.getValue());
107105

108-
switch (definition.getFormat()) {
109-
case BYTE:
110-
buffer.writeByte((byte) value);
111-
break;
112-
case SHORT:
113-
buffer.writeShortLE((short) value);
114-
break;
115-
case INT:
116-
VarInts.writeInt(buffer, (int) value);
117-
break;
118-
case FLOAT:
119-
buffer.writeFloatLE((float) value);
120-
break;
121-
case STRING:
122-
writeString(buffer, (String) value);
123-
break;
124-
case NBT:
125-
this.writeTag(buffer, value);
126-
break;
127-
case VECTOR3I:
128-
writeVector3i(buffer, (Vector3i) value);
129-
break;
130-
case LONG:
131-
VarInts.writeLong(buffer, (long) value);
132-
break;
133-
case VECTOR3F:
134-
writeVector3f(buffer, (Vector3f) value);
135-
break;
136-
default:
137-
throw new UnsupportedOperationException("Unknown entity data type " + definition.getFormat());
106+
// Skip if transformer returns null (indicating this entry shouldn't be serialized)
107+
if (value == null) {
108+
continue;
138109
}
110+
111+
serializedEntries.add(new AbstractMap.SimpleEntry<>(definition, value));
139112
} catch (Exception e) {
140113
throw new IllegalArgumentException("Failed to encode EntityData " + definition.getId() + " of " + definition.getType().getTypeName(), e);
141114
}
142115
}
116+
117+
VarInts.writeUnsignedInt(buffer, serializedEntries.size());
118+
119+
for (Map.Entry<EntityDataTypeMap.Definition<?>, Object> entry : serializedEntries) {
120+
EntityDataTypeMap.Definition<?> definition = entry.getKey();
121+
Object value = entry.getValue();
122+
VarInts.writeUnsignedInt(buffer, definition.getId());
123+
VarInts.writeUnsignedInt(buffer, definition.getFormat().ordinal());
124+
125+
switch (definition.getFormat()) {
126+
case BYTE:
127+
buffer.writeByte((byte) value);
128+
break;
129+
case SHORT:
130+
buffer.writeShortLE((short) value);
131+
break;
132+
case INT:
133+
VarInts.writeInt(buffer, (int) value);
134+
break;
135+
case FLOAT:
136+
buffer.writeFloatLE((float) value);
137+
break;
138+
case STRING:
139+
writeString(buffer, (String) value);
140+
break;
141+
case NBT:
142+
this.writeTag(buffer, value);
143+
break;
144+
case VECTOR3I:
145+
writeVector3i(buffer, (Vector3i) value);
146+
break;
147+
case LONG:
148+
VarInts.writeLong(buffer, (long) value);
149+
break;
150+
case VECTOR3F:
151+
writeVector3f(buffer, (Vector3f) value);
152+
break;
153+
default:
154+
throw new UnsupportedOperationException("Unknown entity data type " + definition.getFormat());
155+
}
156+
}
143157
}
144158

145159
@Override

‎bedrock-codec/src/main/java/org/cloudburstmc/protocol/bedrock/data/entity/EntityDataMap.java‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,38 +12,48 @@
1212

1313
public final class EntityDataMap implements Map<EntityDataType<?>, Object> {
1414

15+
public static EnumMap<EntityFlag, Boolean> flagsOf(EntityFlag... flags) {
16+
EnumMap<EntityFlag, Boolean> map = new EnumMap<>(EntityFlag.class);
17+
for (EntityFlag flag : flags) {
18+
map.put(flag, true);
19+
}
20+
return map;
21+
}
22+
1523
private final Map<EntityDataType<?>, Object> map = new LinkedHashMap<>();
1624

1725
@NonNull
18-
public EnumSet<EntityFlag> getOrCreateFlags() {
19-
EnumSet<EntityFlag> flags = get(FLAGS);
26+
public EnumMap<EntityFlag, Boolean> getOrCreateFlags() {
27+
EnumMap<EntityFlag, Boolean> flags = get(FLAGS);
2028
if (flags == null) {
2129
flags = get(FLAGS_2);
2230
if (flags == null) {
23-
flags = EnumSet.noneOf(EntityFlag.class);
31+
flags = new EnumMap<>(EntityFlag.class);
2432
}
2533
this.putFlags(flags);
2634
}
2735
return flags;
2836
}
2937

30-
public EnumSet<EntityFlag> getFlags() {
38+
public EnumMap<EntityFlag, Boolean> getFlags() {
3139
return get(FLAGS);
3240
}
3341

34-
public EntityFlag setFlag(EntityFlag flag, boolean value) {
42+
public EntityFlag clearFlag(EntityFlag flag) {
3543
Objects.requireNonNull(flag, "flag");
36-
EnumSet<EntityFlag> flags = this.getOrCreateFlags();
37-
if (value) {
38-
flags.add(flag);
39-
} else {
40-
flags.remove(flag);
41-
}
44+
EnumMap<EntityFlag, Boolean> flags = this.getOrCreateFlags();
45+
flags.remove(flag);
46+
return flag;
47+
}
4248

49+
public EntityFlag setFlag(EntityFlag flag, boolean value) {
50+
Objects.requireNonNull(flag, "flag");
51+
EnumMap<EntityFlag, Boolean> flags = this.getOrCreateFlags();
52+
flags.put(flag, value);
4353
return flag;
4454
}
4555

46-
public EnumSet<EntityFlag> putFlags(EnumSet<EntityFlag> flags) {
56+
public EnumMap<EntityFlag, Boolean> putFlags(EnumMap<EntityFlag, Boolean> flags) {
4757
Objects.requireNonNull(flags, "flags");
4858
this.map.put(FLAGS, flags);
4959
this.map.put(FLAGS_2, flags);
@@ -107,7 +117,7 @@ public Object put(EntityDataType<?> key, Object value) {
107117
checkNotNull(value, "value was null for %s", key);
108118
checkArgument(key.isInstance(value), "value with type %s is not an instance of %s", value.getClass(), key);
109119
if (key == FLAGS || key == FLAGS_2) {
110-
return this.putFlags((EnumSet<EntityFlag>) value);
120+
return this.putFlags((EnumMap<EntityFlag, Boolean>) value);
111121
}
112122
return this.map.put(key, value);
113123
}

0 commit comments

Comments
 (0)