Util ReflectedCraftPlayer

Discussion in 'Resources' started by JanTuck, Jan 20, 2017.

Thread Status:
Not open for further replies.
  1. Offline


    I created this due to a player needing a way to use CraftPlayer on diff versions of Bukkit.

    How to create a ReflectedCraftPlayer
    ReflectedCraftPlayer reflectedCraftPlayer = new ReflectedCraftPlayer(Player);

    But some functions returns objects.

    Yes, some functions due to it sending some versioned stuff returns an Object like getHandle().
    I thought about this and added the ReflectedCraftPlayer#sendPacket(Packet) to the class.

    How do i send the packet again?
    Its easy just do something like.
    ReflectedCraftPlayer reflectedCraftPlayer = new ReflectedCraftPlayer( (Player) commandSender );
    Class<?> packetPlayOutChatClass = ReflectionUtil.getNMSClass("PacketPlayOutChat");
    Constructor<?> packetPlayOutChatConstructor = ReflectionUtil.getConstructor(packetPlayOutChatClass, ReflectionUtil.ICHAT_BASE_COMPONENT_CLASS);
    Object IChatBaseComponent_Serializer = ReflectionUtil.invokeMethodReturnObject(ReflectionUtil.ICHAT_BASE_COMPONENT__SERIALIZER_CLASS, null, "a", "{\"text\":\""+ ChatColor.GREEN +"This is a example!\"}");
    Object packetPlayOutChat = ReflectionUtil.getObjectFromConstructor(packetPlayOutChatConstructor, IChatBaseComponent_Serializer);


    There we go @Zombie_Striker anything else i can do to reduce line count?
    Last edited: Jan 20, 2017
  2. Offline

    I Al Istannen

    1. Follow naming conventions!
      "final Class<?> CraftPlayer_Class" does not. Correct would be "CRAFT_PLAYER_CLASS".
    2. "handle.getClass().getField("playerConnection")"
      Finding the fields and methods is what takes time. Cache these as "private static final" variables
    3. "(Boolean)", "(Float)", "(Integer)"
      You can actually cast to the primitives (e.g. "(boolean)"). Looks a bit nicer IMHO.
    4. "// Had some trouble passing the array so just rewrote the method."
      Care to explain the trouble? :)
    5. "getDisplayName" and many other
      You do realize there are API methods for that? I do not understand why you try to invoke those using Reflection. It makes no sense.
    6. "Entity player"
      You take an "Entity" as parameter, call it player and think it is of the class "CraftPlayer"? Take a "Player" and store it as such!
    7. "this.craftPlayer = player;"
      What should this do? Why do you store the Player as "Object", if you know it will be of type "Player"?
    8. Honestly, 90% of the methods can be done via the API. You can instruct your IDE to create delegate methods, so things like RefletedPlayer#getDisplayName are just passed to the wrapped Player object. I do not know why you would need this though.

    1. "getClassName"
      Does the same as Class#getSimpleName. No need for it at all.
    2. "Class"
      For the love of god, stop dealing with raw types.
    3. "getMethod(Class<?> superClass"
      Why is that parameter called superClass?
    4. "Object toInvokeOn"
      That is generally called "handle"
    5. "catch (Exception e) {"
      You shouldn't catch any exception. Catch what you need only, let the rest bubble up. Why should that method swallow every error there might be?
    6. You just take the exception and print it. Rethrowing as a RuntimeException may be nicer. Or provide a mechanism to differentiate a method returning null from a method which returns null. Over on PerceiveCore, I wrote a dedicated ReflectResponse class, taking this job.
    7. "classes = getType(parameters);"
      Nice idea but inherently flawed. What if my method is "public void foo(Integer integer);"? Your code would not find it, as it looks for a method with the parameters "int", not the wrapper class.
    8. Your methods give no feedback. They can throw a few hundred NPEs insid (which your catch with "Exception" catches), and the caller will never know if the operation succeeded.
    9. DRY. Seriously, DRY.
      Have a look at these two methods:
      "invokeMethodReturnObject(Method method, Object toInvokeOn, Object... parameters) {"
      "invokeMethodReturnObject(Class<?> superClass, Object toInvokeOn, String method, Object... parameters)"

      What would be a sensible strategy for the second one? Correct, you obtain the "Method" and invoke the other method, already designed to do the job of actually invoking it. What you currently do is writing the same code over and over.
    10. "getContructer"
      That thing is called a "Constructor", as it constructs a class. Probably a typo, still funny ;)
    11. "String version = Bukkit.getServer().getClass().getPackage().getName().split("\\.")[3];"
      Something, something caching as "private static final String" :)

    These were the things that I saw after skimming over the code. As always, there are probably more.
    AlvinB and Zombie_Striker like this.
  3. Offline


  4. Offline


    What's the point of it if you are still importing nms packages (packet), making it version dependant... ?
  5. Offline


  6. Offline


    You could remove the lines that can be accessed through the Player object, or can be used without needing to import any NMS classes.

    The next step after this, if you wish to add onto what you currently have, would be to make new methods to send out custom packets. Instead of having to do all the reflection by hand, it would be nice to just to have to input a few variables. Here is a list of all the packets:

    These are ones where it would be nice if they had their own methods:
Thread Status:
Not open for further replies.

Share This Page