[Code Snippet] Workaround for the new Bukkit.getOnlinePlayers() method

...so? Developers have to update their code. Would you like to explain how the code you've posted makes it so developers don't have to update their code, because I'm not following.

If we're talking about performance issues, the method no longer has to clone an array every time it's called. There shouldn't be any concurrent modification unless somebody does something stupid.

 

For the last time, the point isn't to exempt devs from updating, it's for the sake of reverse compatibility. It's nice to be able to support two versions of Minecraft at once, which is impossible when compiling against newer Bukkit builds (and please don't go on about how that breaks reverse compatibility anyway, because it doesn't necessarily).

 

Well, neither does this. Necessarily.

 

I guess we got to the point where it is clear why this exists. It is general knowledge that you should go with the plugin version that conforms with the version of Bukkit you are running.

 

This is not the correct way to implement backwards compatibility.

Correct way:

PHP:

public int implPlayerCount() {
    Object playerCollection = Bukkit.getServer().getPlayers();

    if (playerCollection instaceof Collection) return ((Collection) playerCollection).size();
    else return ((Player[]) playerCollection).length;
}

We leave Collection non-typesafe since we never know when <? extends Player> might become <Player>
We could use <?> however not all objects types have to extend java.lang.Object...

 

That doesn't actually work because the compiled bytecode still says "give me the method getOnlinePlayers that takes no arguments and returns a Collection", which won't exist on older versions, back to the original problem.

 

SpaceManiac
My question is, have you actually seen the compiled bytecode for such code?

Here, I'll link you to some representative code:
http://ideone.com/fC0tqz

By the way, since I'm lazy, I just put in the raw value as an Object parameter to getCount and let the compiler typecast for me.

Edit: Also, both print out the value of 2, our expected answer.

 

Code:

Code:

import org.bukkit.Bukkit;
import org.bukkit.entity.Player;
import java.util.Collection;

public class Players {
    public int implPlayerCount() {
        Object playerCollection = Bukkit.getOnlinePlayers();
        if (playerCollection instanceof Collection) return ((Collection) playerCollection).size();
        else return ((Player[]) playerCollection).length;
    }
}

Bytecode: (lines with "-" are compiled on old version, lines with "+" are compiled on new version, lines without are the same on both)

Code:

public class Players
  SourceFile: "Players.java"
  minor version: 0
  major version: 51
  flags: ACC_PUBLIC, ACC_SUPER
Constant pool:
    #1 = Methodref          #7.#18        //  java/lang/Object."<init>":()V
-  #2 = Methodref          #19.#20        //  org/bukkit/Bukkit.getOnlinePlayers:()[Lorg/bukkit/entity/Player;
+  #2 = Methodref          #19.#20        //  org/bukkit/Bukkit.getOnlinePlayers:()Ljava/util/Collection;
    #3 = Class              #21            //  java/util/Collection
    #4 = InterfaceMethodref #3.#22        //  java/util/Collection.size:()I
    #5 = Class              #23            //  "[Lorg/bukkit/entity/Player;"
    #6 = Class              #24            //  Players
    #7 = Class              #25            //  java/lang/Object
    #8 = Utf8              <init>
    #9 = Utf8              ()V
  #10 = Utf8              Code
  #11 = Utf8              LineNumberTable
  #12 = Utf8              implPlayerCount
  #13 = Utf8              ()I
  #14 = Utf8              StackMapTable
  #15 = Class              #25            //  java/lang/Object
  #16 = Utf8              SourceFile
  #17 = Utf8              Players.java
  #18 = NameAndType        #8:#9          //  "<init>":()V
  #19 = Class              #26            //  org/bukkit/Bukkit
-  #20 = NameAndType        #27:#28        //  getOnlinePlayers:()[Lorg/bukkit/entity/Player;
+  #20 = NameAndType        #27:#28        //  getOnlinePlayers:()Ljava/util/Collection;
  #21 = Utf8              java/util/Collection
  #22 = NameAndType        #29:#13        //  size:()I
  #23 = Utf8              [Lorg/bukkit/entity/Player;
  #24 = Utf8              Players
  #25 = Utf8              java/lang/Object
  #26 = Utf8              org/bukkit/Bukkit
  #27 = Utf8              getOnlinePlayers
-  #28 = Utf8              ()[Lorg/bukkit/entity/Player;
+  #28 = Utf8              ()Ljava/util/Collection;
  #29 = Utf8              size
{
  public Players();
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
          0: aload_0
          1: invokespecial #1                  // Method java/lang/Object."<init>":()V
          4: return
      LineNumberTable:
        line 5: 0
 
  public int implPlayerCount();
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=2, args_size=1
-        0: invokestatic  #2                  // Method org/bukkit/Bukkit.getOnlinePlayers:()[Lorg/bukkit/entity/Player;
+        0: invokestatic  #2                  // Method org/bukkit/Bukkit.getOnlinePlayers:()Ljava/util/Collection;
          3: astore_1
          4: aload_1
          5: instanceof    #3                  // class java/util/Collection
          8: ifeq          21
        11: aload_1
        12: checkcast    #3                  // class java/util/Collection
        15: invokeinterface #4,  1            // InterfaceMethod java/util/Collection.size:()I
        20: ireturn
        21: aload_1
        22: checkcast    #5                  // class "[Lorg/bukkit/entity/Player;"
        25: checkcast    #5                  // class "[Lorg/bukkit/entity/Player;"
        28: arraylength
        29: ireturn
      LineNumberTable:
        line 7: 0
        line 8: 4
        line 9: 21
      StackMapTable: number_of_entries = 1
            frame_type = 252 /* append */
              offset_delta = 21
        locals = [ class java/lang/Object ]
 
}

As you can see, when compiled against the old version, the old method is called, and when compiled against the new version, the new method is called. Nothing has been accomplished. My recommendation is to simply build against the oldest Bukkit version you intend to support.

 

SpaceManiac
Ive had this issue in java 6, I thought byte code wasn't the issue but it is D: it still throws an NoSuchMethodException even with using instanceof and Object, I was trying to write a wrapper what mimics an collection but also an normal Array but the thing is whenever the Bukkit.getOnlinePlayers() is compiled with the latest version it indeed expects a collection even when lowering the bukkit version, but I didn't know is it true a higher java version can ignore that?

 

xTrollxDudex NoSuchMethodError for you sir

Code:

[14:26:01 ERROR]: Error occurred while enabling B vbroken (Is it up to date?)
java.lang.NoSuchMethodError: a.A.getOnlinePlayers()[Lorg/bukkit/entity/Player;
    at b.B.implPlayerCount(B.java:22) ~[?:?]
    at b.B.onEnable(B.java:17) ~[?:?]
    at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:250)
    at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:324)
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:405)
    at org.bukkit.craftbukkit.v1_7_R3.CraftServer.loadPlugin(CraftServer.java:467)
    at org.bukkit.craftbukkit.v1_7_R3.CraftServer.enablePlugins(CraftServer.java:385)
    at net.minecraft.server.v1_7_R3.MinecraftServer.n(MinecraftServer.java:359)
    at net.minecraft.server.v1_7_R3.MinecraftServer.g(MinecraftServer.java:333)
    at net.minecraft.server.v1_7_R3.MinecraftServer.a(MinecraftServer.java:289)
    at net.minecraft.server.v1_7_R3.DedicatedServer.init(DedicatedServer.java:193)
    at net.minecraft.server.v1_7_R3.MinecraftServer.run(MinecraftServer.java:450)
    at net.minecraft.server.v1_7_R3.ThreadServerApplication.run(SourceFile:628)

B was build while A.getOnlinePlayers() returned a Player[]. Then I changed A to return a Collection<? extends Player>, rebuild, dumped both in my localhost server and ran it.

 

i already stated - you must have classes compiled for each version and factory to give class for valid version.

also you can assemble bytecode yourself - method return is not part of signature and can be omitted.

 

fireblast709
I was expecting the JIT to be smarter than that :/

RawCode
http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.3.4

Unfortunately, it is. However, I do believe it is is easier to change the entire method ref to the specific return type than to remake the entire thing.

 

xTrollxDudex
JAVA feature large amount of compile time checks and safety measures.
All these checks can be ignored with direct bytecode manipulation.

 

Sorry to dredge up a semi-old thread and maybe sound a little impertinent, but...

I don't think we'd all be as concerned with backwards compatibility if it hadn't been *8 months* since the latest RB.

It's really hard to hear that we "shouldn't" make a plugin that can be backwards and forwards compatible, when many server admins aren't willing to update to a dev or beta build. I like my plugin to support the latest RB, but I also want to support the newest CraftBukkit version for the admins that would prefer people not have to downgrade clients to play. This is becoming harder and harder, and I could see how changes like this would make it nearly impossible.

Personally I'm not using getOnlinePlayers so this particular change isn't an issue to me, but on a general note I think that breaking API changes should be avoided unless there's a recent RB we can build against. I understand and appreciate the challenges the Bukkit team faces, especially with the recent pace of Mojang development- but if the API itself could stay consistent between major versions, that'd help us out a lot- I think?

At this point it feels like we may not ever see an RB for 1.7 - but couldn't changes like this wait for the switchover to 1.8 dev, perhaps?