Fixes #3139-Pause on menu

[Arihant-Joshi]

Contains

Fixes #3139 .

How to test

Pressing Escape in the game pauses the world time.

Outstanding before merging

Maybe the getPlayerCount() method could be a separate file, so that other developers may use it too.
The player count may be updated whenever a player joins/leaves so that the engine doesn't iterate over all clients every time the escape menu is pressed.
Maybe raise a new issue for this?

[Cervator]

Huh, wonder why our build bot didn't catch this. Lets see:

Gooey: Test this please
@GooeyHub: Test this please

Thanks for the PR @Arihant-Joshi :-)

In which ways have you tested this one, curiously? Single player, one player connected to a headless server, server+client + client, 2 players on headless, etc?

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[Arihant-Joshi]

To test for it working(or rather not working if more than one player in the server), I simply changed the line
if(getPlayerCount()==1) {
to
if(getPlayerCount()==2) {
(see commit
89aae6e

The menu stopped working(as the check to pause now was playerCount()==2) when only one player was connected, which is expected behavior(only for that test).

[Arihant-Joshi]

Anyhow there still seems to be an issue with my PR with the game time pausing after closing the inventory by pressing the escape key.
Need some time to work on it.

[Arihant-Joshi]

Fixed the issue with previous commits by changing the check from (ESCAPE) ButtonState.DOWN to
nuiManager.isOpen("engine:pauseMenu")

Also, should I put the code for checking the player count in a different class? If yes, where?

[Cervator]

Heya @Arihant-Joshi. Sorry about the delay, coming out from a period without enough time to really dig into PRs :-)

So on looking at this more closely I see two issues:

1. You've got some trivial whitespace issues that violate our code style standards. Could you please make sure you have Checkstyle working in your IDE and run it before committing code? Easy fix :-)
2. I'm not sure this approach is right. Pausing the game is fair if you're playing in single player mode - but this approach actually also triggers if you're the sole player on a headless server. At that point time pauses locally but continues on the server, leading to some really weird issues

Ideally you should be able to base the check on some different logic, maybe related to whether the network system is live at all. If it isn't it is pretty safe to assume single player mode.

This does actually work fine in legit single player, for instance with JS running you finally wouldn't starve to death just from going AFK. But the implications of checking this way are problematic :-)

[Arihant-Joshi]

I did that so that the player may pause even on multiplayer if he is the only person online.
Should I change the code to only work for singleplayer?

[Arihant-Joshi]

I did away with the approach of counting the number of players on a server, in favor of checking whether the Network Mode is None(Single Player).
Also did the thing using checkstyle :-)

[Cervator]

Thanks! That works well now :-)

The trick in multiplayer is that you have both a client plus a server even if there's only one player active. The client thinks time is paused with the menu up but the server doesn't, resulting in strange behavior as you unpause and the server has been expecting time has in fact passed, and starts trying to update the player accordingly :-)

It would be possible to pause time both on the client and the server. You could even do it if any one player had that menu up but ... at some point that probably gets silly. Fair enough that single player is different, at least for now 👍

Added you to the game credits during the merge! I did remove one unused remaining variable, but that's trivial. Appreciate the style fixes.