Describe the bug
If the EntityType in a packet is not Vanilla but is instead a custom EntityType made using a custom NMS entity registered in the Registry, it will return null when using this function although the data is in the packet.

It just makes it a bit harder to swap these out for people intentionally using a custom EntityType/ID as there's thus no pattern to packets - The best way I found was to just figure out where this would be in a given Packet Type, get it using GetModifier(int) and compare it as a string - Not ideal because the field is different between different packet types, where for a Vanilla EntityType this would be obtained consistently using getEntityTypeModifier instead to check and replace.

I'm not particularly experienced although from looking it seems it's because it uses Minecraft's built-in EntityType and returns a Bukkit entity - My entity would neither be accessible from that EntityType or have a bukkit Entity - perhaps an alternative way to get the NMS EntityType from Registry would be the solution if appropriate

To Reproduce
Steps to reproduce the behavior:

1. Make a custom entity using NMS extending a class (for example, Minecart)
2. Register it in the Registry as an EntityType (for example, railcart)
3. Custom entity passes this EntityType to it's superconstructor instead of substituting (by taking it as an argument when spawned)
4. Set up a PacketListener, for example Entity Spawn
5. Try to use getEntityTypeModifier to get the EntityType
6. Gives null instead of EntityType, useless for checking if it is your custom entity

Expected behavior
I expected it to give the NMS EntityType from Registry although that was my own assumption - as it returns a Bukkit Entity anyway I guess this would be intended behaviour although I thought it was noteworthy despite this.

Screenshots
N/A

Version Info
ProtocolLib install info

Additional context
Only useful with registered custom entities using their own EntityType in superconstructor - Generally they will instead pass a vanilla EntityType and use workarounds to change it back to the custom entity when loaded - Very niche usecase but could have other uses

Hopefully this is appropriate as a bug despite potentially being intended behaviour

Hi,
in that particular case it's a bit hard to make it accessible for everyone, converting to the bukkit enum is the best we can do here. In your case I would suggest using the (relatively new) way to access the registry from ProtocolLib. That api is currently a bit limited to a few methods, but I hope that I can find some time to expand it further. An example use (for your case would be):

Class<?> entityTypeClass = MinecraftReflection.getEntityTypes();
WrappedRegistry registry = WrappedRegistry.getRegistry(entityTypeClass);

Object type = event.getPacket().getModifier().withType(entityTypeClass).read(0);

int typeId = registry.getId(type);
MinecraftKey key = registry.getKey(type);

log.warning("Sending entity type " + key.getFullKey() + " (id: " + typeId + ")");

Would result in something like:

[10:39:00 WARN]: [protocollib] Sending entity type minecraft:bat (id: 5)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:bat (id: 5)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:enderman (id: 23)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:skeleton (id: 81)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:skeleton (id: 81)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:zombie (id: 112)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:zombie (id: 112)
[10:39:00 WARN]: [protocollib] Sending entity type minecraft:zombie (id: 112)
[10:39:01 WARN]: [protocollib] Sending entity type minecraft:skeleton (id: 81)
[10:39:01 WARN]: [protocollib] Sending entity type minecraft:skeleton (id: 81)

Note that during testing for this I found a small issue with how we register registry types which are parameterized (like EntityType) so the code shown is not working until #1628 is merged :/. Sorry for that!

Edit: It's merged 🎉

I realised immediately after posting this the only event that would ever require the EntityType would be Spawn Entity or Spawn Living Entity or the variants (which seem to all be deprecated in 1.19 anyway)

Means just assuming it is pretty safe because it's only one packet that ever requires it - No need to replace it in more packets or have a more general listener which is where this would be useful

Felt I should add this because although it would make it more consistent across versions it relies on NMS stuff to make the entity anyway so it would be one of many things to change and pretty simple - Not very useful