search

Hugobros3 / chunkstories

Somewhat fancy blocky game engine written in Kotlin
http://chunkstories.xyz
Other
221 stars 10 forks source link

NullPointerException issuing a chat command in singleplayer #3

Closed landru27 closed 6 years ago

landru27 commented 6 years ago

alpha 1.0.1 Ubuntu 16.04.1

summary : NullPointerException issuing a chat command

detail : When running in singleplayer mode, issuing a "/tp" command yields a NullPointerException ...

at io.xol.chunkstories.server.LocalServerContext.getPlayerByName(LocalServerContext.java:127) at io.xol.chunkstories.server.commands.player.TpCommand.handleCommand(TpCommand.java:65) at io.xol.chunkstories.plugin.DefaultPluginManager.dispatchCommand(DefaultPluginManager.java:261) at io.xol.chunkstories.gui.layer.ingame.ChatManager.processTextInput(ChatManager.java:232) ... etc ...

It is 'world' in getPlayerByName() that is null.

After tracing it out, my analysis is that when LocalServerContext() is constructed as part of a chain of object instantiation that begins with changeWorld() on line 93 in LevelSelection.java, LocalServerContext() calls client.getWorld() too early. That is, the chain of instantiation that begins with "new WorldClientLocal()" that is the first parameter passed to changeWorld() is still executing; so, changeWorld() has not been called yet, and Client uses changeWorld() to populate 'world' returned by getWorld() that LocalServerContext() calls.

In theory, this bug could be triggered by other chat commands, and/or by other conditions. It should be trigged by anything that calls getPlayerByName(). In practice, it looks like the only other chat command that calls getPlayerByName() is /give, whose syntax in singleplayer mode disallows the player-name argument. When, as a test, I change it so the "/give item qty player" syntax works in singleplayer, without the suggested fix, it does indeed through the same NullPointerException at the same place.

I have a suggested fix for this, which I will issue as a PR right after I submit this issue. My suggested fix supplies 'world' to LocalServerContext() directly, since it is already known, and because it prevents LocalServerContext() from needing to obtain it from the Client.

Hugobros3 commented 6 years ago

I remember having one such issue a few months back. Did you check the bug is present on the latest git master revision ? Seems weird I'd leave something like that in, as this basically makes local debugging impossible

Hugobros3 commented 6 years ago

Okay I get it, you can ommit the [who] in the /tp syntax and the command will assume you're teleporting yourself and won't call getPlayerByName(). I have been able to reproduce the bug by stating my own name in the /tp command. I'll look at your fix, thanks !