search

dmulloy2 / ProtocolLib

Provides read and write access to the Minecraft protocol with Bukkit.
GNU General Public License v2.0
1.04k stars 257 forks source link

NPE while setting Objects into DataWatcher #160

Closed geNAZt closed 8 years ago

geNAZt commented 8 years ago

I get a NPE when i try to use setObject() on a WrappedDataWatcher:

Caused by: java.lang.NullPointerException
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_72]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_72]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_72]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_72]
        at com.comphenix.protocol.reflect.accessors.DefaultMethodAccessor.invoke(DefaultMethodAccessor.java:16) ~[?:?]
        at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:227) ~[?:?]
        at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:219) ~[?:?]
        at net.gommehd.gameapi.util.NameTagSpawner.createArmorStand(NameTagSpawner.java:142) ~[?:?]
        at net.gommehd.gameapi.util.NameTagSpawner.setNameTag(NameTagSpawner.java:124) ~[?:?]
        at net.gommehd.gameapi.util.CoinSpawner.start(CoinSpawner.java:51) ~[?:?]
        at net.gommehd.survivalgames.listener.player.PlayerDeathListener.onDeath(PlayerDeathListener.java:85) ~[?:?]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_72]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_72]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_72]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_72]
        at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot.jar:git-Spigot-unknown-ef13ca4]
        ... 22 more

Line 142 of the NameTagSpawner is:

WrappedDataWatcher wdw = new WrappedDataWatcher();
wdw.setObject( 0, ( byte ) 32 ); // This is line 142
wdw.setObject( 2, message );
wdw.setObject( 3, ( byte ) 1 );

ProtocolLib Version 3.7.0 Build 241

dmulloy2 commented 8 years ago

Try with the newest dev build.

scrayos commented 8 years ago

Still errors:

java.lang.RuntimeException: An internal error occured.
    at com.comphenix.protocol.reflect.accessors.DefaultMethodAccessor.invoke(DefaultMethodAccessor.java:20) ~[?:?]
    at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:308) ~[?:?]
    at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:295) ~[?:?]
    at net.justchunks.jclibrary.objects.hologram.Hologram.<init>(Hologram.java:47) ~[?:?]
    at net.justchunks.jclibrary.handler.gui.HologramHandler.addHologram(HologramHandler.java:32) ~[?:?]
    at net.justchunks.jchub.handler.JoinObjectHandler.initializePVPSword(JoinObjectHandler.java:52) ~[?:?]
    at net.justchunks.jchub.handler.JoinObjectHandler.<init>(JoinObjectHandler.java:24) ~[?:?]
    at net.justchunks.jchub.JCHub.onEnable(JCHub.java:30) ~[?:?]
    at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:291) ~[server.jar:git-Spigot-87e2f47-ef13ca4]
    at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:340) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:405) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at org.bukkit.craftbukkit.v1_9_R1.CraftServer.loadPlugin(CraftServer.java:361) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at org.bukkit.craftbukkit.v1_9_R1.CraftServer.enablePlugins(CraftServer.java:321) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at net.minecraft.server.v1_9_R1.MinecraftServer.t(MinecraftServer.java:411) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at net.minecraft.server.v1_9_R1.MinecraftServer.l(MinecraftServer.java:376) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at net.minecraft.server.v1_9_R1.MinecraftServer.a(MinecraftServer.java:331) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at net.minecraft.server.v1_9_R1.DedicatedServer.init(DedicatedServer.java:269) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at net.minecraft.server.v1_9_R1.MinecraftServer.run(MinecraftServer.java:527) [server.jar:git-Spigot-87e2f47-ef13ca4]
    at java.lang.Thread.run(Thread.java:745) [?:1.8.0_74]
Caused by: java.lang.NullPointerException
    at net.minecraft.server.v1_9_R1.DataWatcher.set(DataWatcher.java:106) ~[server.jar:git-Spigot-87e2f47-ef13ca4]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_74]
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_74]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_74]
    at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_74]
    at com.comphenix.protocol.reflect.accessors.DefaultMethodAccessor.invoke(DefaultMethodAccessor.java:16) ~[?:?]
    ... 18 more
geNAZt commented 8 years ago

Yes still having issues with #244:

Caused by: java.lang.RuntimeException: An internal error occured.
        at com.comphenix.protocol.reflect.accessors.DefaultMethodAccessor.invoke(DefaultMethodAccessor.java:20) ~[?:?]
        at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:308) ~[?:?]
        at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:295) ~[?:?]
        at net.gommehd.gameapi.util.NameTagSpawner.createArmorStand(NameTagSpawner.java:141) ~[?:?]
        at net.gommehd.gameapi.util.NameTagSpawner.setNameTag(NameTagSpawner.java:123) ~[?:?]
        at net.gommehd.gameapi.util.CoinSpawner.start(CoinSpawner.java:51) ~[?:?]
        at net.gommehd.survivalgames.listener.player.PlayerDeathListener.onDeath(PlayerDeathListener.java:85) ~[?:?]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_72]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_72]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_72]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_72]
        at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot.jar:git-Spigot-unknown-73eff0d]
        ... 22 more
Caused by: java.lang.NullPointerException
        at net.minecraft.server.v1_9_R1.DataWatcher.set(DataWatcher.java:106) ~[spigot.jar:git-Spigot-unknown-73eff0d]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_72]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_72]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_72]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_72]
        at com.comphenix.protocol.reflect.accessors.DefaultMethodAccessor.invoke(DefaultMethodAccessor.java:16) ~[?:?]
        at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:308) ~[?:?]
        at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:295) ~[?:?]
        at net.gommehd.gameapi.util.NameTagSpawner.createArmorStand(NameTagSpawner.java:141) ~[?:?]
        at net.gommehd.gameapi.util.NameTagSpawner.setNameTag(NameTagSpawner.java:123) ~[?:?]
        at net.gommehd.gameapi.util.CoinSpawner.start(CoinSpawner.java:51) ~[?:?]
        at net.gommehd.survivalgames.listener.player.PlayerDeathListener.onDeath(PlayerDeathListener.java:85) ~[?:?]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_72]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_72]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_72]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_72]
        at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot.jar:git-Spigot-unknown-73eff0d]
        ... 22 more
geNAZt commented 8 years ago

Some more debug digging:

I implemented our ObjectDumper and created a state Dump from the WrappedDataWatcher when the Exception is about to happen:

WrappedDataWatcher:
  Statics={
    HANDLE_TYPE=class net.minecraft.server.v1_9_R1.DataWatcher; 
    GETTER=null; 
    SETTER=DefaultMethodAccessor [
      method=public void net.minecraft.server.v1_9_R1.DataWatcher.set(net.minecraft.server.v1_9_R1.DataWatcherObject,java.lang.Object)
    ]; 
    ENTITY_FIELD=null; 
    MAP_FIELD=null; 
    ENTITY_DATA_FIELD=null; 
    constructor=DefaultConstrutorAccessor [
      constructor=public net.minecraft.server.v1_9_R1.DataWatcher(net.minecraft.server.v1_9_R1.Entity)
    ]; 
    lightningConstructor=DefaultConstrutorAccessor [
      constructor=public net.minecraft.server.v1_9_R1.EntityLightning(net.minecraft.server.v1_9_R1.World,double,double,double,boolean)
    ]; 
    fakeEntity=EntityLightning['unknown'/34, l='~NULL~', x=0.00, y=0.00, z=0.00]
  }

AbstractWrapper: 
  Fields={
    handle=net.minecraft.server.v1_9_R1.DataWatcher@43e75e29; 
    handleType=class net.minecraft.server.v1_9_R1.DataWatcher
  }

It seems that all needed Fields and Objects are there to manipulate the DataWatcher. Now we look into the DataWatcher itself:

   public <T> void set(DataWatcherObject<T> datawatcherobject, T t0) {
        DataWatcher.Item datawatcher_item = this.c(datawatcherobject);

        if (ObjectUtils.notEqual(t0, datawatcher_item.b())) { // This is line 106
            datawatcher_item.a(t0);
            this.b.a(datawatcherobject);
            datawatcher_item.a(true);
            this.f = true;
        }

    }

So the error occures when you want to set a DataWatcherObject which has not been registered before.

So some sort of using DataWatcher#register(DataWatcherObject, Object); and catching its IllegalArgumentException to check if its already registered and if so use DataWatcher#set(DataWatcherObject, Object); would solve this issue

Paulomart commented 8 years ago

This will fix it. It still needs some caching. It would be possilbe to have a Set with the currently registered types. Not sure how @dmulloy2 wants this.

        try {
            Accessors.getMethodAccessor(handleType, "register", watcherObject.getHandleType(), Object.class).invoke(handle, watcherObject.getHandle(), WrappedWatchableObject.getUnwrapped(value));
        } catch (IllegalArgumentException e) {
            e.printStackTrace();
        } catch (Exception e) {
            e.printStackTrace();
        }
dmulloy2 commented 8 years ago

Yeah I'm aware of this. Basically the best course of action is for developers to use the entity's data watcher instead of creating your own. The reason for this is that mojang tied keys (data watcher objects) to entity classes, so instead of it just being an id, it's an object in the entity class.

scrayos commented 8 years ago

Unfortunately for my approach there is no "default datawatcher" of the entity because I create completely virtual armorstands for holograms.

dmulloy2 commented 8 years ago

Ah right, I was looking for a use case of an entity-less data watcher. I'll work on a fix.

scrayos commented 8 years ago 
java.lang.IllegalArgumentException: You must specify a serializer to register an object!
    at org.apache.commons.lang.Validate.notNull(Validate.java:192) ~[server.jar:git-Spigot-b39373b-b6bb6be]
    at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:348) ~[?:?]
    at com.comphenix.protocol.wrappers.WrappedDataWatcher.setObject(WrappedDataWatcher.java:316) ~[?:?]
    at net.justchunks.jclibrary.objects.hologram.Hologram.<init>(Hologram.java:47) ~[?:?]
    at net.justchunks.jclibrary.handler.gui.HologramHandler.addHologram(HologramHandler.java:32) ~[?:?]
    at net.justchunks.jchub.handler.JoinObjectHandler.initializePVPSword(JoinObjectHandler.java:52) ~[?:?]
    at net.justchunks.jchub.handler.JoinObjectHandler.<init>(JoinObjectHandler.java:24) ~[?:?]
    at net.justchunks.jchub.JCHub.onEnable(JCHub.java:30) ~[?:?]
    at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:291) ~[server.jar:git-Spigot-b39373b-b6bb6be]
    at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:340) [server.jar:git-Spigot-b39373b-b6bb6be]
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:405) [server.jar:git-Spigot-b39373b-b6bb6be]
    at org.bukkit.craftbukkit.v1_9_R1.CraftServer.loadPlugin(CraftServer.java:361) [server.jar:git-Spigot-b39373b-b6bb6be]
    at org.bukkit.craftbukkit.v1_9_R1.CraftServer.enablePlugins(CraftServer.java:321) [server.jar:git-Spigot-b39373b-b6bb6be]
    at net.minecraft.server.v1_9_R1.MinecraftServer.t(MinecraftServer.java:411) [server.jar:git-Spigot-b39373b-b6bb6be]
    at net.minecraft.server.v1_9_R1.MinecraftServer.l(MinecraftServer.java:376) [server.jar:git-Spigot-b39373b-b6bb6be]
    at net.minecraft.server.v1_9_R1.MinecraftServer.a(MinecraftServer.java:331) [server.jar:git-Spigot-b39373b-b6bb6be]
    at net.minecraft.server.v1_9_R1.DedicatedServer.init(DedicatedServer.java:269) [server.jar:git-Spigot-b39373b-b6bb6be]
    at net.minecraft.server.v1_9_R1.MinecraftServer.run(MinecraftServer.java:527) [server.jar:git-Spigot-b39373b-b6bb6be]
    at java.lang.Thread.run(Thread.java:745) [?:1.8.0_74]

This is thrown now, with the same code I used before.

Paulomart commented 8 years ago

Then commenting out the validation the metadata still dosent work

dmulloy2 commented 8 years ago

In 1.9, Mojang added a bunch of overhead to the data watchers. Indexes are now wrapped in DataWatcherObjects, which contain the index and a DataWatcherSerializer. Serializers are in the DataWatcherRegistry. WatchableObjects were replaced by DataWatcher Items, which contain the watcher object, the value, and the dirty state.

ProtocolLib mimics the structure as much as it can. Basically, you should use the data watcher objects instead of indexes if you want to register new objects. Using just the index works fine, so long as an item already exists at that index. See the WrappedDataWatcher and WrappedWatchableObject for more.

Everything should be resolved at this point. If not, create a new issue here.

scrayos commented 8 years ago

Still unresolved as you can see from my comments in your commit: https://github.com/dmulloy2/ProtocolLib/commit/5115bcaf98ca0a53497096bcf1e924f39dd62ca6 Of course I tested it before.

The problem is pretty much is not backwards-compatible as there's no workaround for the default setObject(int, Object) and that I don't know where to get a serializer for all these things/Which kind of serializers are meant here.

dmulloy2 commented 8 years ago 
Entity entity = packet.getEntityModifier(event).read(0);
WrappedDataWatcher watcher = WrappedDataWatcher.getEntityWatcher(entity);

Serializer serializer = Registry.get(String.class);
WrappedDataWatcherObject object = new WrappedDataWatcherObject(0, serializer);
watcher.setObject(object, "Hello, World!");
scrayos commented 8 years ago

Ah, that helps a lot, although it's way more "bloated" compared to 1.8. But yeah, that's how Mojang does things.

dmulloy2 commented 8 years ago

Yeah unfortunately that's the smallest I could get it, the same bloat exists in NMS.

scrayos commented 8 years ago

There are no errors anymore, but the hologram is still not shown.

WrapperPlayServerSpawnEntityLiving armorStand = new WrapperPlayServerSpawnEntityLiving();
armorStand.setEntityId(entityID);
armorStand.setType(30); //Armorstand-NetworkID
armorStand.setX((int) (location.getX() * 32));
armorStand.setY((int) (location.getY() * 32));
armorStand.setZ((int) (location.getZ() * 32));

WrappedDataWatcher.Serializer stringSerializer = WrappedDataWatcher.Registry.get(String.class);
WrappedDataWatcher.Serializer byteSerializer = WrappedDataWatcher.Registry.get(Byte.class);

WrappedDataWatcher wdw = new WrappedDataWatcher();
wdw.setObject(new WrappedDataWatcher.WrappedDataWatcherObject(0, byteSerializer), (byte) (0x20)); //Invisibility
wdw.setObject(new WrappedDataWatcher.WrappedDataWatcherObject(2, stringSerializer), text); //CustomName
wdw.setObject(new WrappedDataWatcher.WrappedDataWatcherObject(3, byteSerializer), (byte) 1); //CustomName-Visibility
wdw.setObject(new WrappedDataWatcher.WrappedDataWatcherObject(10, byteSerializer), (byte) (0x01 | 0x08 | 0x16)); //Small, Without Baseplate, Marker
armorStand.setMetadata(wdw);
Paulomart commented 8 years ago

Your location is wrong. It's now an double, not a fixed point number. Look at wiki.vg

scrayos commented 8 years ago

Thanks! - I looked there and noted a few other changes, but that one has slipped through :)

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.