Skip to content

Commit 1be94aa

Browse files
committed
Rework packet type deprecation to actually work properly
Also fix compatibility with 1.8.0
1 parent ea7900d commit 1be94aa

File tree

10 files changed

+221
-168
lines changed

10 files changed

+221
-168
lines changed

modules/API/src/main/java/com/comphenix/protocol/PacketType.java

Lines changed: 87 additions & 72 deletions
Large diffs are not rendered by default.

modules/API/src/main/java/com/comphenix/protocol/reflect/ObjectEnum.java renamed to modules/API/src/main/java/com/comphenix/protocol/PacketTypeEnum.java

Lines changed: 57 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,18 @@
1515
* 02111-1307 USA
1616
*/
1717

18-
package com.comphenix.protocol.reflect;
18+
package com.comphenix.protocol;
1919

2020
import java.lang.reflect.Field;
2121
import java.lang.reflect.Modifier;
2222
import java.util.HashSet;
2323
import java.util.Iterator;
2424
import java.util.Set;
2525

26+
import com.comphenix.protocol.PacketType;
2627
import com.google.common.collect.BiMap;
2728
import com.google.common.collect.HashBiMap;
29+
import com.google.common.collect.Sets;
2830

2931
/**
3032
* Represents a more modern object-based enum.
@@ -33,40 +35,53 @@
3335
* want to prevent the creation of additional members dynamically.
3436
* @author Kristian
3537
*/
36-
public class ObjectEnum<T> implements Iterable<T> {
38+
public class PacketTypeEnum implements Iterable<PacketType> {
3739
// Used to convert between IDs and names
38-
protected BiMap<T, String> members = HashBiMap.create();
40+
protected Set<PacketType> members = Sets.newHashSet();
3941

4042
/**
41-
* Registers every declared integer field.
42-
* @param fieldType Field type
43+
* Registers every declared PacketType field.
4344
*/
44-
public ObjectEnum(Class<T> fieldType) {
45-
registerAll(fieldType);
45+
public PacketTypeEnum() {
46+
registerAll();
4647
}
4748

4849
/**
4950
* Registers every public assignable static field as a member.
50-
* @param fieldType Field type
5151
*/
5252
@SuppressWarnings("unchecked")
53-
protected void registerAll(Class<T> fieldType) {
53+
protected void registerAll() {
5454
try {
5555
// Register every non-deprecated field
5656
for (Field entry : this.getClass().getFields()) {
57-
if (Modifier.isStatic(entry.getModifiers()) && fieldType.isAssignableFrom(entry.getType())) {
58-
T value = (T) entry.get(null);
59-
60-
if (value == null)
61-
throw new IllegalArgumentException("Field " + entry + " was NULL. Remember to " +
62-
"construct the object after the field has been declared.");
63-
registerMember(value, entry.getName());
57+
if (Modifier.isStatic(entry.getModifiers()) && PacketType.class.isAssignableFrom(entry.getType())) {
58+
PacketType value = (PacketType) entry.get(null);
59+
if (value == null) {
60+
throw new IllegalArgumentException("Field " + entry.getName() + " was null!");
61+
}
62+
63+
value.setName(entry.getName());
64+
65+
if (entry.getAnnotation(PacketType.ForceAsync.class) != null) {
66+
value.forceAsync();
67+
}
68+
69+
boolean deprecated = entry.getAnnotation(Deprecated.class) != null;
70+
if (deprecated) value.setDeprecated();
71+
72+
if (members.contains(value)) {
73+
// Replace potentially deprecated packet types with non-deprecated ones
74+
if (!deprecated) {
75+
members.remove(value);
76+
members.add(value);
77+
}
78+
} else {
79+
members.add(value);
80+
}
6481
}
6582
}
66-
} catch (IllegalArgumentException e) {
67-
e.printStackTrace();
68-
} catch (IllegalAccessException e) {
69-
e.printStackTrace();
83+
} catch (Exception ex) {
84+
ex.printStackTrace();
7085
}
7186
}
7287

@@ -76,11 +91,14 @@ protected void registerAll(Class<T> fieldType) {
7691
* @param name - name of member.
7792
* @return TRUE if the member was registered, FALSE otherwise.
7893
*/
79-
public boolean registerMember(T instance, String name) {
80-
if (!members.containsKey(instance)) {
81-
members.put(instance, name);
94+
public boolean registerMember(PacketType instance, String name) {
95+
instance.setName(name);
96+
97+
if (!members.contains(instance)) {
98+
members.add(instance);
8299
return true;
83100
}
101+
84102
return false;
85103
}
86104

@@ -89,38 +107,36 @@ public boolean registerMember(T instance, String name) {
89107
* @param member - the member to check.
90108
* @return TRUE if the given member has been registered, FALSE otherwise.
91109
*/
92-
public boolean hasMember(T member) {
93-
return members.containsKey(member);
110+
public boolean hasMember(PacketType member) {
111+
return members.contains(member);
94112
}
95113

96114
/**
97115
* Retrieve a member by name,
98116
* @param name - name of member to retrieve.
99117
* @return The member, or NULL if not found.
118+
* @deprecated Don't use this
100119
*/
101-
public T valueOf(String name) {
102-
return members.inverse().get(name);
103-
}
104-
105-
/**
106-
* Retrieve the name of the given member.
107-
* @param member - the member to retrieve.
108-
* @return Declared name of the member, or NULL if not found.
109-
*/
110-
public String getDeclaredName(T member) {
111-
return members.get(member);
120+
@Deprecated
121+
public PacketType valueOf(String name) {
122+
for (PacketType member : members) {
123+
if (member.name().equals(name))
124+
return member;
125+
}
126+
127+
return null;
112128
}
113-
129+
114130
/**
115131
* Retrieve every registered member.
116132
* @return Enumeration of every value.
117133
*/
118-
public Set<T> values() {
119-
return new HashSet<T>(members.keySet());
134+
public Set<PacketType> values() {
135+
return new HashSet<>(members);
120136
}
121137

122138
@Override
123-
public Iterator<T> iterator() {
124-
return members.keySet().iterator();
139+
public Iterator<PacketType> iterator() {
140+
return members.iterator();
125141
}
126142
}

modules/API/src/main/java/com/comphenix/protocol/ProtocolLogger.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,10 @@ public static void debug(String message, Object... args) {
7373
log("[Debug] " + message, args);
7474
}
7575
}
76+
77+
public static void debug(String message, Throwable ex) {
78+
if (isDebugEnabled()) {
79+
plugin.getLogger().log(Level.WARNING, "[Debug] " + message, ex);
80+
}
81+
}
7682
}

modules/API/src/main/java/com/comphenix/protocol/injector/netty/NettyProtocolRegistry.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ public NettyProtocolRegistry() {
4040

4141
@Override
4242
protected synchronized void initialize() {
43-
ProtocolLogger.debug("Initializing the Netty protocol registry"); // Debug for issue #202
44-
4543
Object[] protocols = enumProtocol.getEnumConstants();
4644

4745
// ID to Packet class maps
@@ -75,8 +73,7 @@ protected synchronized void initialize() {
7573
result.containers.add(new MapContainer(map));
7674
}
7775

78-
for (int i = 0; i < protocols.length; i++) {
79-
Object protocol = protocols[i];
76+
for (Object protocol : protocols) {
8077
Enum<?> enumProtocol = (Enum<?>) protocol;
8178
Protocol equivalent = Protocol.fromVanilla(enumProtocol);
8279

@@ -103,8 +100,8 @@ protected void associatePackets(Register register, Map<Integer, Class<?>> lookup
103100
register.serverPackets.add(type);
104101
if (sender == Sender.CLIENT)
105102
register.clientPackets.add(type);
106-
} catch (IllegalArgumentException ex) {
107-
// Sometimes this happens with fake packets, just ignore it
103+
} catch (Exception ex) {
104+
ProtocolLogger.debug("Encountered an exception associating packet " + type, ex);
108105
}
109106
}
110107
}

modules/API/src/main/java/com/comphenix/protocol/injector/packet/PacketRegistry.java

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -171,16 +171,7 @@ public static Set<PacketType> getServerPacketTypes() {
171171
initialize();
172172
NETTY.synchronize();
173173

174-
Set<PacketType> types = new HashSet<>();
175-
176-
// Filter out unsupported packets
177-
for (PacketType type : NETTY.getServerPackets()) {
178-
if (!type.isDeprecated()) {
179-
types.add(type);
180-
}
181-
}
182-
183-
return types;
174+
return NETTY.getServerPackets();
184175
}
185176

186177
/**
@@ -207,16 +198,7 @@ public static Set<PacketType> getClientPacketTypes() {
207198
initialize();
208199
NETTY.synchronize();
209200

210-
Set<PacketType> types = new HashSet<>();
211-
212-
// Filter out unsupported packets
213-
for (PacketType type : NETTY.getClientPackets()) {
214-
if (!type.isDeprecated()) {
215-
types.add(type);
216-
}
217-
}
218-
219-
return types;
201+
return NETTY.getClientPackets();
220202
}
221203

222204
/**

modules/API/src/main/java/com/comphenix/protocol/reflect/FuzzyReflection.java

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717

1818
package com.comphenix.protocol.reflect;
1919

20-
import java.lang.reflect.Constructor;
21-
import java.lang.reflect.Field;
22-
import java.lang.reflect.Method;
23-
import java.lang.reflect.Modifier;
20+
import java.lang.reflect.*;
2421
import java.util.ArrayList;
2522
import java.util.Arrays;
2623
import java.util.LinkedHashSet;
@@ -421,6 +418,30 @@ public List<Field> getFieldListByType(Class<?> type) {
421418

422419
return fields;
423420
}
421+
422+
/**
423+
* Retrieves a field with a given type and parameters. This is most useful
424+
* when dealing with Collections.
425+
*
426+
* @param fieldType Type of the field
427+
* @param params Variable length array of type parameters
428+
* @return The field
429+
*
430+
* @throws IllegalArgumentException If the field cannot be found
431+
*/
432+
public Field getParameterizedField(Class<?> fieldType, Class<?>... params) {
433+
for (Field field : getFields()) {
434+
if (field.getType().equals(fieldType)) {
435+
Type type = field.getGenericType();
436+
if (type instanceof ParameterizedType) {
437+
if (Arrays.equals(((ParameterizedType) type).getActualTypeArguments(), params))
438+
return field;
439+
}
440+
}
441+
}
442+
443+
throw new IllegalArgumentException("Unable to find a field with type " + fieldType + " and params " + Arrays.toString(params));
444+
}
424445

425446
/**
426447
* Retrieve the first field that matches.

modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ public final class PacketFilterManager implements ListenerInvoker, InternalManag
8383
new ReportType("%s doesn't depend on ProtocolLib. Check that its plugin.yml has a 'depend' directive.");
8484

8585
// Registering packet IDs that are not supported
86-
public static final ReportType REPORT_UNSUPPORTED_SERVER_PACKET_ID = new ReportType("[%s] Unsupported server packet ID in current Minecraft version: %s");
87-
public static final ReportType REPORT_UNSUPPORTED_CLIENT_PACKET_ID = new ReportType("[%s] Unsupported client packet ID in current Minecraft version: %s");
86+
public static final ReportType REPORT_UNSUPPORTED_SERVER_PACKET = new ReportType("[%s] Unsupported server packet in current Minecraft version: %s");
87+
public static final ReportType REPORT_UNSUPPORTED_CLIENT_PACKET = new ReportType("[%s] Unsupported client packet in current Minecraft version: %s");
8888

8989
// Problems injecting and uninjecting players
9090
public static final ReportType REPORT_CANNOT_UNINJECT_PLAYER = new ReportType("Unable to uninject net handler for player.");
@@ -624,7 +624,7 @@ private void enablePacketFilters(PacketListener listener, Iterable<PacketType> p
624624
playerInjection.addPacketHandler(type, listener.getSendingWhitelist().getOptions());
625625
else
626626
reporter.reportWarning(this,
627-
Report.newBuilder(REPORT_UNSUPPORTED_SERVER_PACKET_ID).messageParam(PacketAdapter.getPluginName(listener), type)
627+
Report.newBuilder(REPORT_UNSUPPORTED_SERVER_PACKET).messageParam(PacketAdapter.getPluginName(listener), type)
628628
);
629629
}
630630

@@ -634,7 +634,7 @@ private void enablePacketFilters(PacketListener listener, Iterable<PacketType> p
634634
packetInjector.addPacketHandler(type, listener.getReceivingWhitelist().getOptions());
635635
else
636636
reporter.reportWarning(this,
637-
Report.newBuilder(REPORT_UNSUPPORTED_CLIENT_PACKET_ID).messageParam(PacketAdapter.getPluginName(listener), type)
637+
Report.newBuilder(REPORT_UNSUPPORTED_CLIENT_PACKET).messageParam(PacketAdapter.getPluginName(listener), type)
638638
);
639639
}
640640
}

modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ProtocolInjector.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.bukkit.plugin.Plugin;
3030

3131
import com.comphenix.protocol.PacketType;
32+
import com.comphenix.protocol.ProtocolLogger;
3233
import com.comphenix.protocol.concurrency.PacketTypeSet;
3334
import com.comphenix.protocol.error.ErrorReporter;
3435
import com.comphenix.protocol.error.Report;
@@ -132,12 +133,15 @@ public synchronized void inject() {
132133
if (serverConnection != null) {
133134
break;
134135
}
135-
} catch (Exception e) {
136-
// Try the next though
137-
e.printStackTrace();
136+
} catch (Exception ex) {
137+
ProtocolLogger.debug("Encountered an exception invoking " + method, ex);
138138
}
139139
}
140140

141+
if (serverConnection == null) {
142+
throw new ReflectiveOperationException("Failed to obtain server connection");
143+
}
144+
141145
// Handle connected channels
142146
final ChannelInboundHandler endInitProtocol = new ChannelInitializer<Channel>() {
143147
@Override
@@ -153,8 +157,8 @@ protected void initChannel(final Channel channel) throws Exception {
153157
injectionFactory.fromChannel(channel, ProtocolInjector.this, playerFactory).inject();
154158
}
155159
}
156-
} catch (Exception e) {
157-
reporter.reportDetailed(ProtocolInjector.this, Report.newBuilder(REPORT_CANNOT_INJECT_INCOMING_CHANNEL).messageParam(channel).error(e));
160+
} catch (Exception ex) {
161+
reporter.reportDetailed(ProtocolInjector.this, Report.newBuilder(REPORT_CANNOT_INJECT_INCOMING_CHANNEL).messageParam(channel).error(ex));
158162
}
159163
}
160164
};
@@ -183,21 +187,22 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
183187
FuzzyReflection fuzzy = FuzzyReflection.fromObject(serverConnection, true);
184188

185189
try {
186-
List<Field> fields = fuzzy.getFieldListByType(List.class);
187-
for (Field field : fields) {
188-
ParameterizedType param = (ParameterizedType) field.getGenericType();
189-
if (param.getActualTypeArguments()[0].equals(MinecraftReflection.getNetworkManagerClass())) {
190-
field.setAccessible(true);
191-
networkManagers = (List<Object>) field.get(serverConnection);
192-
}
193-
}
190+
Field field = fuzzy.getParameterizedField(List.class, MinecraftReflection.getNetworkManagerClass());
191+
field.setAccessible(true);
192+
193+
networkManagers = (List<Object>) field.get(serverConnection);
194194
} catch (Exception ex) {
195-
networkManagers = (List<Object>) fuzzy.getMethodByParameters("getNetworkManagers", List.class, serverConnection.getClass())
196-
.invoke(null, serverConnection);
195+
ProtocolLogger.debug("Encountered an exception checking list fields", ex);
196+
197+
Method method = fuzzy.getMethodByParameters("getNetworkManagers", List.class,
198+
new Class<?>[] { serverConnection.getClass() });
199+
method.setAccessible(true);
200+
201+
networkManagers = (List<Object>) method.invoke(null, serverConnection);
197202
}
198203

199204
if (networkManagers == null) {
200-
throw new RuntimeException("Failed to obtain list of network managers.");
205+
throw new ReflectiveOperationException("Failed to obtain list of network managers");
201206
}
202207

203208
// Insert ProtocolLib's connection interceptor
@@ -376,7 +381,7 @@ public boolean uninjectPlayer(InetSocketAddress address) {
376381

377382
@Override
378383
public void addPacketHandler(PacketType type, Set<ListenerOptions> options) {
379-
if (options != null && !type.forceAsync() && !options.contains(ListenerOptions.ASYNC))
384+
if (!type.isAsyncForced() && (options == null || !options.contains(ListenerOptions.ASYNC)))
380385
mainThreadFilters.addType(type);
381386
super.addPacketHandler(type, options);
382387
}

0 commit comments

Comments
 (0)