Support mtasa:// protocol connect args #3211

Lpsd · 2023-10-15T06:54:02Z

Changes

Allows arguments to be supplied to the mtasa:// protocol and received by the server in Lua via onPlayerConnect event parameters.
Adds an extra argument to redirectPlayer which allows you to pass a table to achieve the same result (all keys, values are automatically converted to strings, in order to form the 'argument path').
A fix for some of the SharedUtil tests (Use MTA temp path for client tests #3209) has been modified and merged into this PR.

Additional Info

Total length of arguments is limited to 1024 including / delimiter per key/value pair (arguments are treated as path string for 99% of lifetime)

Subject to change for typical url parameters e.g ?a=1&b=2

This feature is useful for the Discord RPC functionality, servers can create a party system using this or give rewards for joining via Discord (of course this can extend to other platforms).

Example

Have serverside script running:

SERVER_IP, SERVER_PORT, SERVER_PASS = "localhost", 22003, ""
DEFAULT_ARGS = {["foo"] = "abc", ["bar"] = "def"}

local storedArgs

function connect(nick, ip, username, serial, version, versionString, args)
    storedArgs = args
    iprint(args)
end
addEventHandler("onPlayerConnect", root, connect)

function redirect(player)
    redirectPlayer(player, SERVER_IP, SERVER_PORT, SERVER_PASS, (type(storedArgs) == "table") and storedArgs or DEFAULT_ARGS)
end
addCommandHandler("redirect", redirect)

Action	Result
Player connects with `mtasa://123.456.78.9:22003/foo/123/bar/456/baz/789`
Player uses `redirect` command	Same as above

Notes

To test mtasa:// protocol with debug build, edit registry Computer\HKEY_CLASSES_ROOT\mtasa\shell\open\command and set to your mtasa build path e.g "C:\mtasa-blue\Bin\Multi Theft Auto_d.exe"%1

To-do

Handle / in user provided keys/values, either remove character or URL encode? (/ is our delimiter so would affect converting to pairs)
- / will be removed from user provided args
Fix argument parser issue when using std::unordered_map<type, type> for Lua args
- See Fix std::unordered_map<std::string, std::string> parsing #3246
Determine whether keys/values (protocol parameters) should be changed to ?a=1&b=2 format

Supplied to server via onPlayerConnect event parameters

Inder00 · 2023-10-19T11:43:48Z

marge

Server/mods/deathmatch/logic/CPlayer.h

Client/core/CCommandFuncs.cpp

Client/core/CCore.cpp

Client/core/CCore.h

fresholia · 2023-11-04T12:33:17Z

Also it should work with connect console command, as I see i can't connect the game with connect command.

Fernando-A-Rocha · 2023-11-04T14:48:23Z

What about also changing redirectPlayer?

Fernando-A-Rocha · 2023-11-16T21:13:49Z

well done, lgtm

Lpsd · 2023-11-16T21:17:29Z

the redirectPlayer functionality has uncovered an issue with the argument parser which needs resolving before this can be merged. See discussion on dev discord

Lpsd · 2023-11-16T21:38:45Z

Also it should work with connect console command, as I see i can't connect the game with connect command.

Works fine for me, can you confirm how you're using the command and what errors you're seeing? @fresholia

Lpsd · 2023-11-18T06:02:13Z

We can't use std::unordered_map<std::string, std::string> to retrieve table arguments in a Lua definition (because a script providing the wrong type ie {1, 2, 3} will cause a crash, and I couldn't find a solution for this).

For this reason I've changed to use CLuaArgument in the unordered_map, as it can handle any value from the Lua side, then convert this to a string using the already available CLuaArgument::GetAsString method

However, I did have to implement a hash function for CLuaArgument in order to use it with std::unordered_map - could do with some opinions on this; not sure if it's correct (approach and implementation-wise). This is an issue we inevitably need to tackle at some point anyway (accepting tables of any value using new arg parser)...

mtasa-blue/Client/mods/deathmatch/logic/lua/CLuaArgument.h

Lines 68 to 78 in c96a8ff

    
           struct Hash 
        
           { 
        
               size_t operator()(const CLuaArgument& a) const 
        
               { 
        
                   SString str; 
        
                   if (!a.GetAsString(str))            // If we can't convert to a string, then use pointer address instead 
        
                       str = std::to_string((unsigned long long)(void**)&a); 
        
                   return std::hash<int>{}(a.GetType()) ^ (std::hash<std::string>{}(str) << 1); 
        
               } 
        
           };

Shared/sdk/SharedUtil.Misc.hpp

Server/mods/deathmatch/logic/packets/CPlayerJoinDataPacket.cpp

Client/mods/deathmatch/logic/lua/CLuaArgument.cpp

Client/core/CCore.cpp

tederis · 2023-11-19T04:48:07Z

We can't use std::unordered_map<std::string, std::string> to retrieve table arguments in a Lua definition (because a script providing the wrong type ie {1, 2, 3} will cause a crash, and I couldn't find a solution for this).

For this reason I've changed to use CLuaArgument in the unordered_map, as it can handle any value from the Lua side, then convert this to a string using the already available CLuaArgument::GetAsString method

However, I did have to implement a hash function for CLuaArgument in order to use it with std::unordered_map - could do with some opinions on this; not sure if it's correct (approach and implementation-wise). This is an issue we inevitably need to tackle at some point anyway (accepting tables of any value using new arg parser)...

mtasa-blue/Client/mods/deathmatch/logic/lua/CLuaArgument.h

Lines 68 to 78 in c96a8ff

struct Hash

{

size_t operator()(const CLuaArgument& a) const

{

SString str;

if (!a.GetAsString(str)) // If we can't convert to a string, then use pointer address instead

str = std::to_string((unsigned long long)(void**)&a);

return std::hash<int>{}(a.GetType()) ^ (std::hash<std::string>{}(str) << 1);

}

};

I think it would be better to fix a crash instead of constructing hash related complicated things. I hope my PR #3246 will help.

This reverts commit c96a8ff.

tederis · 2023-12-01T11:37:29Z

Client/core/CCore.h

@@ -286,6 +288,9 @@ class CCore : public CCoreInterface, public CSingleton<CCore>
    bool   IsUsingCustomStreamingMemorySize();
    size_t GetStreamingMemory();

+    void               SetProtocolConnectArgs(const std::string&& args) { m_strProtocolConnectArgs = args; }


Unfortunately, const rvalue reference doesn't make sense. Even without a const qualifier it wont work as expected because in the function's body args is actually lvalue reference. So you should remove const and cast args to the rvalue explicitly: SetProtocolConnectArgs(std::string&& args) { m_strProtocolConnectArgs = std::move(args); }

I agree with this.
Also, that custom hash function for CLuaArgument isn't good at all, and should be avoided at all costs.
Actually, and kind of comparasion/hashing of it should be avoided.

tederis · 2023-12-01T11:50:27Z

Client/sdk/core/CCoreInterface.h

@@ -190,6 +190,9 @@ class CCoreInterface
    virtual bool   IsUsingCustomStreamingMemorySize() = 0;
    virtual size_t GetStreamingMemory() = 0;

+    virtual void        SetProtocolConnectArgs(const std::string&& args) = 0;


The same: const rvalue has no sense

tederis · 2023-12-01T11:57:12Z

Server/mods/deathmatch/logic/packets/CPlayerJoinDataPacket.h

@@ -44,6 +44,9 @@ class CPlayerJoinDataPacket final : public CPacket

    bool IsOptionalUpdateInfoRequired() { return m_bOptionalUpdateInfoRequired; }

+    const char* GetConnectArgs() { return m_strConnectArgs; };


Why don't you return a string object reference?

tederis · 2023-12-01T11:58:41Z

Server/mods/deathmatch/logic/CGame.cpp

@@ -1735,6 +1735,11 @@ void CGame::Packet_PlayerJoinData(CPlayerJoinDataPacket& Packet)
        pPlayer->SetBitStreamVersion(Packet.GetBitStreamVersion());
        g_pNetServer->SetClientBitStreamVersion(Packet.GetSourceSocket(), Packet.GetBitStreamVersion());

+        const char* connectArgs = Packet.GetConnectArgs();


This related to a CPlayerJoinDataPacket class problem: it's probably better to use a string.

tederis · 2023-12-01T12:19:01Z

Server/mods/deathmatch/logic/CPlayer.h

@@ -335,6 +335,9 @@ class CPlayer final : public CPed, public CClient
    const SString& GetQuitReasonForLog() { return m_strQuitReasonForLog; }
    void           SetQuitReasonForLog(const SString& strReason) { m_strQuitReasonForLog = strReason; }

+    void           SetConnectArgs(const SString&& args) { m_strConnectArgs = args; }


The same problem with const rvalue ref

tederis · 2023-12-01T12:27:18Z

Server/mods/deathmatch/logic/CGame.cpp

+        const char* connectArgs = Packet.GetConnectArgs();
+
+        if (std::strlen(connectArgs) > 0)
+            pPlayer->SetConnectArgs(std::move(Packet.GetConnectArgs()));


I would recommend to remove std::move here because Packet_PlayerJoinData(and even CGame) method doesn't own this string.

tederis · 2023-12-01T12:39:34Z

Client/mods/deathmatch/logic/rpc/CInputRPCs.cpp

@@ -240,6 +240,20 @@ void CInputRPCs::ForceReconnect(NetBitStreamInterface& bitStream)

        if (bitStream.Read(usPort))
        {
+            char* szArgs = new char[MAX_PROTOCOL_CONNECT_ARGS_LENGTH + 1];


It seems that we have a memory leak here. Because szArgs isn't deleted below.
Maybe a preallocated std::string is the solution? It's null terminated and guaranteed to contiguous since C++11 and we have non-const ::data member since C++17(please, use it).

But it's even better to use bitStream.ReadString() (with the modification of a packet itself).

Dutchman101 · 2024-01-28T23:50:18Z

Here's the last status update on this PR:

Dutchman101 — 01/07/2024 1:42 AM
If you address the final Dec 1 code reviews on #3211, it can be merged

lopsi — 01/07/2024 3:04 PM
there is more to do than that
I would like to use other arg format, ?foo=bar&baz=boz
I know it's possible, I already started working on it, but ran into few issues
MTA updater also uses this format in some weird way, I would like to change it too

Nico8340 · 2024-04-07T18:41:12Z

Any updates on this? Would be a really nice feature.

Lpsd added 7 commits October 15, 2023 07:48

Support mtasa:// protocol connect args

40865c7

Supplied to server via onPlayerConnect event parameters

Fix SharedUtil::Explode declaration

0f93dc5

Increase MAX_PROTOCOL_CONNECT_ARGS_LENGTH to 1024

9a7cc2c

Set nick if empty

be092ca

Update CCoreInterface.h

58ce667

Update CClientGame.cpp

8fdab6d

Remove any usage

ba17a53

tederis reviewed Oct 28, 2023

View reviewed changes

Merge branch 'master' into mtasa-protocol-args

9f05cd8

Lpsd mentioned this pull request Nov 16, 2023

Use MTA temp path for client tests #3209

Closed

Lpsd added 4 commits November 16, 2023 06:00

Address code review

69671b1

Merge fix from multitheftauto#3209

962d7cd

Update fix for SharedUtil.Tests.hpp

5b3849a

Add similar functionality to redirectPlayer via argument

ed7f1cb

Lpsd added 2 commits November 18, 2023 05:47

Allow CLuaArgument usage in std::unordered_map

c96a8ff

Update redirectPlayer args handling

ea878ef

Lpsd added 4 commits November 18, 2023 06:08

Fix compilation error

3398882

Addendum to 3398882

1c4092b

Remove debug code from SharedUtil.Tests.hpp

682e87e

Remove obsolete copy

975a6f1

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Shared/sdk/SharedUtil.Misc.hpp Outdated Show resolved Hide resolved

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Server/mods/deathmatch/logic/packets/CPlayerJoinDataPacket.cpp Outdated Show resolved Hide resolved

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Client/mods/deathmatch/logic/lua/CLuaArgument.cpp Outdated Show resolved Hide resolved

TheNormalnij reviewed Nov 18, 2023

View reviewed changes

Client/core/CCore.cpp Outdated Show resolved Hide resolved

tederis mentioned this pull request Nov 19, 2023

Fix std::unordered_map<std::string, std::string> parsing #3246

Merged

Lpsd added 12 commits November 20, 2023 23:44

Merge branch 'master' into mtasa-protocol-args

9735872

Revert ea878ef (Update redirectPlayer args handling)

9798f5a

Revert "Allow CLuaArgument usage in std::unordered_map"

38831b2

This reverts commit c96a8ff.

Modify table to string processing

64716d9

Remove redundant qualification from declaration

db90a35

Check bitstream version in CPlayerJoinDataPacket

dc3f322

Update curl path in client core project

9e24a06

Change Explode to Split

25809fc

Update CPlayerJoinDataPacket.cpp

5483850

Update CLuaPlayerDefs.cpp

b2f3bed

Modify string replacement in CLuaPlayerDefs::ArgsToString

eb9d516

Remove unnecessary include

d7d597d

tederis reviewed Dec 1, 2023

View reviewed changes

StrixG added the enhancement New feature or request label Dec 21, 2023

Merge branch 'master' into mtasa-protocol-args

adfc201

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support mtasa:// protocol connect args #3211

Support mtasa:// protocol connect args #3211

Lpsd commented Oct 15, 2023 •

edited

Loading

Inder00 commented Oct 19, 2023

fresholia commented Nov 4, 2023

Fernando-A-Rocha commented Nov 4, 2023

Fernando-A-Rocha commented Nov 16, 2023

Lpsd commented Nov 16, 2023 •

edited

Loading

Lpsd commented Nov 16, 2023

Lpsd commented Nov 18, 2023 •

edited

Loading

tederis commented Nov 19, 2023

tederis Dec 1, 2023

Pirulax Dec 26, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023

tederis Dec 1, 2023 •

edited

Loading

tederis Dec 2, 2023

Dutchman101 commented Jan 28, 2024

Nico8340 commented Apr 7, 2024

		@@ -44,6 +44,9 @@ class CPlayerJoinDataPacket final : public CPacket

		bool IsOptionalUpdateInfoRequired() { return m_bOptionalUpdateInfoRequired; }

		const char* GetConnectArgs() { return m_strConnectArgs; };

Support mtasa:// protocol connect args #3211

Are you sure you want to change the base?

Support mtasa:// protocol connect args #3211

Conversation

Lpsd commented Oct 15, 2023 • edited Loading

Changes

Additional Info

Example

Notes

To-do

Inder00 commented Oct 19, 2023

fresholia commented Nov 4, 2023

Fernando-A-Rocha commented Nov 4, 2023

Fernando-A-Rocha commented Nov 16, 2023

Lpsd commented Nov 16, 2023 • edited Loading

Lpsd commented Nov 16, 2023

Lpsd commented Nov 18, 2023 • edited Loading

tederis commented Nov 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tederis Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dutchman101 commented Jan 28, 2024

Nico8340 commented Apr 7, 2024

Lpsd commented Oct 15, 2023 •

edited

Loading

Lpsd commented Nov 16, 2023 •

edited

Loading

Lpsd commented Nov 18, 2023 •

edited

Loading

tederis Dec 1, 2023 •

edited

Loading