Fix cloning in 1.17

Addresses #1222
This commit is contained in:
Dan Mulloy 2021-06-15 23:58:40 -04:00
parent 42bec5a858
commit 9b54794f6b
No known key found for this signature in database
GPG Key ID: 2B62F7DACFF133E8
6 changed files with 84 additions and 69 deletions

View File

@ -1,6 +1,8 @@
language: java
os: linux
dist: focal
jdk:
- openjdk8
- openjdk16
script: mvn clean test
install: true
notifications:

View File

@ -35,6 +35,7 @@ import com.comphenix.protocol.reflect.cloning.*;
import com.comphenix.protocol.reflect.cloning.AggregateCloner.BuilderParameters;
import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
import com.comphenix.protocol.reflect.instances.DefaultInstances;
import com.comphenix.protocol.reflect.instances.InstanceProvider;
import com.comphenix.protocol.utility.MinecraftMethods;
import com.comphenix.protocol.utility.MinecraftReflection;
import com.comphenix.protocol.utility.MinecraftVersion;
@ -86,7 +87,7 @@ public class PacketContainer implements Serializable {
// Used to clone packets
private static final AggregateCloner DEEP_CLONER = AggregateCloner
.newBuilder()
.instanceProvider(DefaultInstances.DEFAULT)
.instanceProvider(StructureCache::newPacket)
.andThen(BukkitCloner.class)
.andThen(ImmutableDetector.class)
.andThen(JavaOptionalCloner.class)
@ -97,7 +98,7 @@ public class PacketContainer implements Serializable {
private static final AggregateCloner SHALLOW_CLONER = AggregateCloner
.newBuilder()
.instanceProvider(DefaultInstances.DEFAULT)
.instanceProvider(StructureCache::newPacket)
.andThen(param -> {
if (param == null)
throw new IllegalArgumentException("Cannot be NULL.");
@ -110,8 +111,12 @@ public class PacketContainer implements Serializable {
.build();
// Packets that cannot be cloned by our default deep cloner
private static final Set<PacketType> CLONING_UNSUPPORTED = Sets.newHashSet(
PacketType.Play.Server.UPDATE_ATTRIBUTES, PacketType.Status.Server.SERVER_INFO);
private static final Set<PacketType> FAST_CLONE_UNSUPPORTED = Sets.newHashSet(
PacketType.Play.Server.BOSS,
PacketType.Play.Server.ADVANCEMENTS,
PacketType.Play.Client.USE_ENTITY,
PacketType.Status.Server.SERVER_INFO
);
/**
* Creates a packet container for a new packet.
@ -1156,13 +1161,15 @@ public class PacketContainer implements Serializable {
public PacketContainer deepClone() {
Object clonedPacket = null;
// Fall back on the alternative (but slower) method of reading and writing back the packet
if (!CLONING_UNSUPPORTED.contains(type)) {
if (!FAST_CLONE_UNSUPPORTED.contains(type)) {
try {
clonedPacket = DEEP_CLONER.clone(getHandle());
} catch (Exception ignored) {}
} catch (Exception ex) {
FAST_CLONE_UNSUPPORTED.add(type);
}
}
// Fall back on the slower alternative method of reading and writing back the packet
if (clonedPacket == null) {
clonedPacket = SerializableCloner.clone(this).getHandle();
}

View File

@ -26,7 +26,6 @@ import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.injector.packet.PacketRegistry;
import com.comphenix.protocol.reflect.StructureModifier;
import com.comphenix.protocol.reflect.compiler.BackgroundCompiler;
import com.comphenix.protocol.reflect.compiler.CompileListener;
import com.comphenix.protocol.reflect.compiler.CompiledStructureModifier;
import com.comphenix.protocol.reflect.instances.DefaultInstances;
import com.comphenix.protocol.utility.MinecraftReflection;
@ -41,9 +40,28 @@ import net.minecraft.network.PacketDataSerializer;
*/
public class StructureCache {
// Structure modifiers
private static ConcurrentMap<PacketType, StructureModifier<Object>> structureModifiers = new ConcurrentHashMap<>();
private static final ConcurrentMap<PacketType, StructureModifier<Object>> structureModifiers = new ConcurrentHashMap<>();
private static Set<PacketType> compiling = new HashSet<>();
private static final Set<PacketType> compiling = new HashSet<>();
public static Object newPacket(Class<?> clazz) {
Object result = DefaultInstances.DEFAULT.create(clazz);
// TODO make these generic
if (result == null) {
try {
return clazz.getConstructor(PacketDataSerializer.class).newInstance(new PacketDataSerializer(new ZeroBuffer()));
} catch (ReflectiveOperationException ex) {
try {
return clazz.getConstructor(PacketDataSerializer.class).newInstance(new ZeroPacketDataSerializer());
} catch (ReflectiveOperationException ex1) {
throw new IllegalArgumentException("Failed to create packet: " + clazz, ex);
}
}
}
return result;
}
/**
* Creates an empty Minecraft packet of the given type.
@ -52,25 +70,8 @@ public class StructureCache {
*/
public static Object newPacket(PacketType type) {
Class<?> clazz = PacketRegistry.getPacketClassFromType(type, true);
// Check the return value
if (clazz != null) {
// TODO: Optimize DefaultInstances
Object result = DefaultInstances.DEFAULT.create(clazz);
if (result == null) {
try {
return clazz.getConstructor(PacketDataSerializer.class).newInstance(new PacketDataSerializer(new ZeroBuffer()));
} catch (ReflectiveOperationException ex) {
try {
return clazz.getConstructor(PacketDataSerializer.class).newInstance(new ZeroPacketDataSerializer());
} catch (ReflectiveOperationException ex1) {
throw new IllegalArgumentException("Failed to create packet for type: " + type, ex);
}
}
}
return result;
return newPacket(clazz);
}
throw new IllegalArgumentException("Cannot find associated packet class: " + type);
}
@ -115,13 +116,13 @@ public class StructureCache {
* @return A structure modifier.
*/
public static StructureModifier<Object> getStructure(final PacketType type, boolean compile) {
Preconditions.checkNotNull(type);
Preconditions.checkNotNull(type, "type cannot be null");
StructureModifier<Object> result = structureModifiers.get(type);
// We don't want to create this for every lookup
if (result == null) {
// Use the vanilla class definition
final StructureModifier<Object> value = new StructureModifier<Object>(
final StructureModifier<Object> value = new StructureModifier<>(
PacketRegistry.getPacketClassFromType(type, true), MinecraftReflection.getPacketClass(), true);
result = structureModifiers.putIfAbsent(type, value);
@ -139,12 +140,8 @@ public class StructureCache {
final BackgroundCompiler compiler = BackgroundCompiler.getInstance();
if (!compiling.contains(type) && compiler != null) {
compiler.scheduleCompilation(result, new CompileListener<Object>() {
@Override
public void onCompiled(StructureModifier<Object> compiledModifier) {
structureModifiers.put(type, compiledModifier);
}
});
compiler.scheduleCompilation(result,
compiledModifier -> structureModifiers.put(type, compiledModifier));
compiling.add(type);
}
}

View File

@ -117,12 +117,11 @@ public class ObjectWriter {
}
// Copy private fields underneath
Class<?> superclass = commonType.getSuperclass();
if (superclass != null && !superclass.equals(Object.class)) {
copyToInternal(source, destination, superclass, false);
}
// Class<?> superclass = commonType.getSuperclass();
//
// if (superclass != null && !superclass.equals(Object.class)) {
// copyToInternal(source, destination, superclass, false);
// }
} catch (FieldAccessException e) {
throw new RuntimeException("Unable to copy fields from " + commonType.getName(), e);
}

View File

@ -256,11 +256,11 @@ public class AggregateCloner implements Cloner {
if (index < cloners.size()) {
Cloner cloner = cloners.get(index);
try {
// try {
return cloner.clone(source);
} catch (Exception ex) {
throw new RuntimeException("Failed to clone " + source + " (" + source.getClass() + ") with " + cloner, ex);
}
// } catch (Exception ex) {
// throw new RuntimeException("Failed to clone " + source + " (" + source.getClass() + ") with " + cloner, ex);
// }
}
// Damn - failure

View File

@ -413,7 +413,7 @@ public class PacketContainerTest {
}
field.setAccessible(true);
assertEquals(field.get(snapshot), field.get(clonedSnapshot));
testEquality(field.get(snapshot), field.get(clonedSnapshot));
} catch (AssertionError e) {
throw e;
} catch (Throwable ex) {
@ -611,8 +611,27 @@ public class PacketContainerTest {
assertArrayEquals(components, back);
}
private void assertPacketsEqual(PacketContainer constructed, PacketContainer cloned) {
StructureModifier<Object> firstMod = constructed.getModifier(), secondMod = cloned.getModifier();
assertEquals(firstMod.size(), secondMod.size());
if (PacketType.Status.Server.SERVER_INFO.equals(constructed.getType())) {
assertArrayEquals(SerializationUtils.serialize(constructed), SerializationUtils.serialize(cloned));
} else {
// Make sure all the fields are equivalent
for (int i = 0; i < firstMod.size(); i++) {
if (firstMod.getField(i).getType().isArray())
assertArrayEquals(getArray(firstMod.read(i)), getArray(secondMod.read(i)));
else
testEquality(firstMod.read(i), secondMod.read(i));
}
}
}
@Test
public void testDeepClone() {
public void testCloning() {
boolean failed = false;
// Try constructing all the packets
for (PacketType type : PacketType.values()) {
if (type.isDeprecated() || type.name().contains("CUSTOM_PAYLOAD") || type.name().contains("TAGS") || !type.isSupported()
@ -644,28 +663,19 @@ public class PacketContainerTest {
//constructed.getModifier().write(1, TEST_COMPONENT);
}
// Clone the packet
PacketContainer cloned = constructed.deepClone();
// Clone the packet both ways
PacketContainer shallowCloned = constructed.shallowClone();
assertPacketsEqual(constructed, shallowCloned);
// Make sure they're equivalent
StructureModifier<Object> firstMod = constructed.getModifier(), secondMod = cloned.getModifier();
assertEquals(firstMod.size(), secondMod.size());
if (PacketType.Status.Server.SERVER_INFO.equals(type)) {
assertArrayEquals(SerializationUtils.serialize(constructed), SerializationUtils.serialize(cloned));
} else {
// Make sure all the fields are equivalent
for (int i = 0; i < firstMod.size(); i++) {
if (firstMod.getField(i).getType().isArray())
assertArrayEquals(getArray(firstMod.read(i)), getArray(secondMod.read(i)));
else
testEquality(firstMod.read(i), secondMod.read(i));
}
}
PacketContainer deepCloned = constructed.deepClone();
assertPacketsEqual(constructed, deepCloned);
} catch (Exception ex) {
ex.printStackTrace();
failed = true;
}
}
assertFalse("Packet(s) failed to clone", failed);
}
// Convert to objects that support equals()