Don't frame as error when player is disconnected using core.disconnect_player

[CrazyladMT]

Fixes #14986

Simply changes the form when a player is disconnected using core.disconnect_player to say "The server has denied access" rather than the misleading "An error occurred"

To do

idk

How to test

1. run /lua core.disconnect_player(me:get_player_name(), "some reason text")
2. observe that an error does not occur, and it shows "The server has denied access"

[sfan5]

now we get the inverse bug: if some kind of error occurs, the dialog says "The server has denied access", which is a lie.

[CrazyladMT]

why is that needed in general, since the other text (which allows for a reason) already kinda says what's up...

[DragonWrangler1]

why is that needed in general, since the other text (which allows for a reason) already kinda says what's up...

[CrazyladMT]

IMHO, I don't see a problem with this:

This is indeed not an error.

if some kind of error occurs

how could I cause such an error (that isn't ModError) for testing purposes?

[sfan5]

Okay, how can I cause such an error (that isn't ModError) for testing purposes?

diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp
--- a/src/network/clientpackethandler.cpp
+++ b/src/network/clientpackethandler.cpp
@@ -359,7 +359,7 @@ void Client::handleCommand_TimeOfDay(NetworkPacket* pkt)
        time_of_day      = time_of_day % 24000;
 
        float time_speed;
-       *pkt >> time_speed;
+       *pkt >> time_speed >> time_speed;
 
        // Update environment
        m_env.setTimeOfDay(time_of_day);

why is that needed in general, since the other text (which allows for a reason) already kinda says what's up...

You could argue that it provides context, since the error message will be in English, which the user might not speak (well), or some technical stuff, which normal people might not understand.

[Wuzzy2]

I agree with sfan5, the old text for actual server errors should not be removed. The server can disconnect you both with a normal function call (disconnect, ban, kick, etc.) but also on a server error.

This message field should not make incorrect claims, i.e. say that an error occured when none occured, or that you were disconnected when actually an error occured (which would be technically true but it would be missing the point).

Does Luanti have any technical distinction of the type of disconnect reason (like: mod error, server error, disconnected by function call, etc.) or is it currently all just strings?

[appgurueu]

Does Luanti have any technical distinction of the type of disconnect reason

Yes, see src/network/networkprotocol.h:

enum AccessDeniedCode : u8 {
	SERVER_ACCESSDENIED_WRONG_PASSWORD,
	SERVER_ACCESSDENIED_UNEXPECTED_DATA,
	SERVER_ACCESSDENIED_SINGLEPLAYER,
	SERVER_ACCESSDENIED_WRONG_VERSION,
	SERVER_ACCESSDENIED_WRONG_CHARS_IN_NAME,
	SERVER_ACCESSDENIED_WRONG_NAME,
	SERVER_ACCESSDENIED_TOO_MANY_USERS,
	SERVER_ACCESSDENIED_EMPTY_PASSWORD,
	SERVER_ACCESSDENIED_ALREADY_CONNECTED,
	SERVER_ACCESSDENIED_SERVER_FAIL,
	SERVER_ACCESSDENIED_CUSTOM_STRING,
	SERVER_ACCESSDENIED_SHUTDOWN,
	SERVER_ACCESSDENIED_CRASH,
	SERVER_ACCESSDENIED_MAX,
};

core.disconnect_player uses SERVER_ACCESSDENIED_CUSTOM_STRING. On the network level it's there.

The problem is that this is eventually thrown away when going through the abstractions, only the string is preserved by client->accessDeniedReason(), and that is put in an "error message" string in Game::checkConnection which probably ends up in gamedata.errormessage.

[CrazyladMT]

Does Luanti have any technical distinction of the type of disconnect reason (like: mod error, server error, disconnected by function call, etc.) or is it currently all just strings?

From my understanding, the Lua side only recieves errormessage and reconnect_requested, and thus cannot distinguish between different disconnect reasons. The only special case Lua can detect is a ModError, because the message contains the substring “ModError”.

[sfan5]

Could replicate the same hack and add "AuthError" to such errors so they can be detected in the main menu.

[CrazyladMT]

Could replicate the same hack and add "AuthError" to such errors

that doesn't sound like a good long-term solution. though it would work for certain errors previously mentioned here:

Yes, see src/network/networkprotocol.h:

enum AccessDeniedCode : u8 {
    SERVER_ACCESSDENIED_WRONG_PASSWORD,
    SERVER_ACCESSDENIED_UNEXPECTED_DATA,
    SERVER_ACCESSDENIED_SINGLEPLAYER,
    SERVER_ACCESSDENIED_WRONG_VERSION,
    SERVER_ACCESSDENIED_WRONG_CHARS_IN_NAME,
    SERVER_ACCESSDENIED_WRONG_NAME,
    SERVER_ACCESSDENIED_TOO_MANY_USERS,
    SERVER_ACCESSDENIED_EMPTY_PASSWORD,
    SERVER_ACCESSDENIED_ALREADY_CONNECTED,
    SERVER_ACCESSDENIED_SERVER_FAIL,
    SERVER_ACCESSDENIED_CUSTOM_STRING,
    SERVER_ACCESSDENIED_SHUTDOWN,
    SERVER_ACCESSDENIED_CRASH,
    SERVER_ACCESSDENIED_MAX,
};

like wrong password, wrong chars in name, empty password, etc, it would introduce more misleading messages when core.disconnect() is called, which goes back to SERVER_ACCESSDENIED_CUSTOM_STRING, which is clearly not an authentication error

maybe Luanti could expose the actual AccessDeniedCode to Lua in the gamedata table, perhaps gamedata.access_denied_code? I am not entirely sure how this would be implemented though

[appgurueu]

Check out struct MainMenuDataForScript in guiMainMenu.h, ScriptApiMainMenu::setMainMenuData, the aforementioned accessDeniedReason etc. and follow some references to determine the path you need to extend.

Do you have a C++ language server like clangd set up? Otherwise this could be a bit tedious; it would involve a lot of manual greping.