Better handle failure to find spawn position / harden for multiplayer

[Cervator]

During today's play test we managed to finally completely crash a server, which has been rare lately. Initially I thought maybe it could be related to a weird interaction between the broken crouching (#2651 / #2565) and character prediction issues (#2358, #2469, #2468) but on digging into the stacktrace I found a todo that could explain the crash much more simply:

13:49:33.397 [main] WARN  o.t.l.c.ServerCharacterPredictionSystem - Received too much input from EntityRef{id = 176010, netId = 6382, prefab = 'engine:player'}, dropping input.
13:49:33.416 [main] ERROR o.terasology.engine.TerasologyEngine - Uncaught exception, attempting clean game shutdown
java.util.NoSuchElementException: No value present
	at java.util.Optional.get(Optional.java:135)
	at org.terasology.logic.players.PlayerFactory.newInstance(PlayerFactory.java:76)
	at org.terasology.logic.players.PlayerSystem.spawnPlayer(PlayerSystem.java:203)
	at org.terasology.logic.players.PlayerSystem.restoreCharacter(PlayerSystem.java:152)
	at org.terasology.logic.players.PlayerSystem.update(PlayerSystem.java:86)
	at org.terasology.engine.modes.StateIngame.update(StateIngame.java:160)
	at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:413)
	at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:368)
	at org.terasology.engine.Terasology.main(Terasology.java:141)
13:49:33.416 [main] INFO  o.terasology.engine.TerasologyEngine - Shutting down Terasology...
13:49:34.254 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Creating game snapshot
13:49:34.256 [main] INFO  o.t.p.i.ReadWriteStorageManager - Saving - Snapshot created: Writing phase starts
13:49:34.287 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: Valery
13:49:34.290 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: mdj117
13:49:34.291 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: Halamix2
13:49:34.292 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: Cervator691
13:49:34.294 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: oal
13:49:34.295 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: Vizaxo
13:49:34.296 [main] INFO  o.t.n.internal.NetworkSystemImpl - Client disconnected: smsunarto
13:49:34.301 [main] INFO  o.t.n.internal.NetworkSystemImpl - Network shutdown

On line 76 in PlayerFactory there is a TODO about handling the lack of a value from the Optional. Which is exactly what took down the entire server. That's a bit much.

Chances are what caused the crash was @smsunarto logging in a third client that somehow failed to find a spawn point. Oddly his first two clients somehow ended up inhabiting the same monkey head. Very peculiar. Maybe from not varying the -homedir ? Would be good to know if that was the case!

But regardless of whether a quirky setup caused the failure to find a spawn point that shouldn't take down the entire server. We should gracefully disconnect that one client if that spawn logic finds no value, offering a helpful error message, then let the server continue on its merry path.

[SurajGoel]

@Cervator Can I have a look at this ?

[SurajGoel]

@Cervator I was thinking maybe we could try few more times to find a spawn position, before disconnecting the entity. What do you say ?

[Cervator]

Much belated reply, sorry about missing your comments @SurajGoel :-)

Depending on spawn position strategy it may be possible to retry a few times. Other times spawn strategy may require always using 0,0,0 which would fail if for some reason that chunk has broken or otherwise gone inaccessible.

I'm not sure if it is easy to judge whether or not a spawn point finding strategy would benefit from retrying. If it isn't obvious in the code probably the client should just gracefully disconnect and present the user with an error message.