Rochet2 / AIO

AIO is a pure lua server-client communication system for Eluna and WoW.
http://rochet2.github.io/AIO
GNU General Public License v2.0
98 stars 41 forks source link

AIO client addon is not working with 1.12.x client #12

Open TheSkullbot opened 3 years ago

TheSkullbot commented 3 years ago

The 1.12.x client seems to be using some sort of 5.0 Lua subset (can't find the exact version).

Some language features in the client AIO addon are using lua 5.1 thus trigerring lua errors in the client.

So far :

Server-side, I'm using cmangos-classic and I had to make some modifications to Eluna for it to work with the latest revision, but maybe it should be another issue. I managed to make it work, so I might be able to provide a pull request for that.

Rochet2 commented 3 years ago

If you have issues with some part, post the error and the code and I may be able to help with it.

TheSkullbot commented 3 years ago

Thanks for your fast answer :) Somehow, I managed to get rid of LUA errors in the client.

But it seems the SendAddonMessage function has changed since 1.12 And I can't really see what needs to be modifier on either side to work with this https://wowpedia.fandom.com/wiki/1.12.0_consolidated_changes_(Iriel)

Right now, the KaevStats example does not work when triggered with the .para command. I can see the server sending the addon message as follow :

WorldSocket::SendAddonMessage - Message : {{1,"Kaev","ShowAttributes"}} WorldSocket::SendAddonMessage - Prefix : SAIO WorldSocket::SendAddonMessage - Channel : 7 WorldSocket::SendAddonMessage - Player : Skullbot (1) WorldSocket::SendAddonMessage - Sender : Skullbot (1)

But nothing seems to be received by the client. Maybe it has to do with the channel 7 ?

Rochet2 commented 3 years ago

Try change 2560 to 255 here https://github.com/Rochet2/AIO/blob/d06da5d8e7c32e45fde93a1632e9e33105dfd895/AIO_Client/AIO.lua#L261 just to make sure its not affecting anything.

Did you try printing right here to see if the message is even received at all? https://github.com/Rochet2/AIO/blob/d06da5d8e7c32e45fde93a1632e9e33105dfd895/AIO_Client/AIO.lua#L1007

What about here to see if the events are even registered? https://github.com/Rochet2/AIO/blob/d06da5d8e7c32e45fde93a1632e9e33105dfd895/AIO_Client/AIO.lua#L1017

Maybe it has to do with the channel 7 ?

The whisper type may indeed be a/the problem. It seems according to https://wowwiki-archive.fandom.com/wiki/API_SendAddonMessage it was introduced in 2.1, so it shouldn't yet exist in 1.12. Maybe should try experiment with some simple messages and try to send/receive them.

The "WHISPER" mode was added with patch 2.1, as servers will begin to rate throttle regular whispers to alleviate spam problems.

Seems mangoszero for example has a setting to enable/disable addon messages. Make sure its off: https://github.com/mangoszero/server/blob/e45aa74e0fa3a9677d90fc012b8161e1e9935742/src/game/WorldHandlers/ChatHandler.cpp#L119

TheSkullbot commented 3 years ago

To answer you, the OnAddonMessage event handler is registered, but not triggerred. In fact, it doesn't even get to Init.

So far, the issue seems to be related to varargs. After some (approximate) tweaks, I managed to get up to the msgmt:Add call, with "AIO", "Init" and "1.74" as params. The problem is that next, these varargs param are used inside the select :

function msgmt:Add(Name, ...)
    assert(Name, "#1 Block must have name")

    print( "msgmt:Add" )
    print( Name ) -- Contains "AIO"
    for i = 1, table.getn(arg) do
        print( arg[i] ) 
        -- arg[1] = "Init"
        -- arg[2] = "1.74"
    end

    self.params[ table.getn(self.params) + 1 ] = {select('#', ... ), Name, ... } -- the ... operator in function call triggers a syntax error in 1.12 client
    self.assemble = true
    return self
end

I'm not sure what this line does so I can't "fix" it. There many other places in the AIO Client code using those varargs, and I think they might cause issues too. I may give arg[1], arg[2], arg[3], ..., arg[20] to those methods using varargs to cover all possible args but I'm pretty sure it won't work.

About config, AddonChannel is enabled in my cmangos config, should I disable it ? Also changed 2560 to 255 as follows :

local AIO_MsgLen = (AIO_SERVER and 255 or 255) -1 - string.len(AIO_ServerPrefix) - string.len(AIO_ShortMsg)
TheSkullbot commented 3 years ago

After further testing, and commenting out this line in msgmt:Add :

self.params[ table.getn(self.params) + 1 ] = {select('#', ... ), Name, ... }

I managed to retrieve the data in ONADDONMSG using argX, but not when using named parameters. This is the code I have and the produced result when calling .para command (thus sending an addon msg from server)

    -- A client side event handler
    -- Passes the incoming message to AIO message handler if it is valid
    local function ONADDONMSG(self, event, prefix, msg, Type, sender)

        print("ONADDONMSG - named params")
        print(self, event, prefix, msg, Type, sender)
        print("ONADDONMSG - argX")
        print(arg1,arg2,arg3,arg4,arg5,arg6,arg7,arg8,arg9)

        if prefix == AIO_ServerPrefix then
            if event == "CHAT_MSG_ADDON" and sender == UnitName("player") then
                -- Normal AIO message handling from addon messages
                AIO_HandleIncomingMsg(msg, sender)
            end
        end
    end

2021-06-12 23_35_48-World of Warcraft

It's not perfect, but I have most of the data sent by the server. Thus it seems a bit wonky to do it this way, but I can't figure out way to use varargs correctly (not sure it's caused by this though)

Still, it's really weird to me how thoses values got in arg1, arg2 etc. Sometimes I have to cycle throuh arg[] as a table, and sometime thoses vars just spawn from nowhere. Maybe it's a LUA thing, I'm not really used to it to find out by myself =/

EDIT : Okay, looks like select is not part of the 5.0 spec 🤦 That explains why I had to comment it out If you have any idea on how to replace this ... 🙏

Rochet2 commented 3 years ago

using argX, but not when using named parameters.

The remaining event arguments are placed within the vararg expression (...), as well as in the arg1-arg9 variables (the latter mechanic is deprecated and will be removed in Cataclysm[1]).

Looks like its wow specific https://wowwiki-archive.fandom.com/wiki/Handling_events looks a bit odd though. Id expect the arguments to work normally for the function, but guess its just how wow API worked back in classic .. hmm...

If you have any idea on how to replace this ... 🙏

The end of the 2.5.8 section describes how to use ... in 5.0. (look for vararg) https://www.lua.org/manual/5.0/manual.html#2.5.8 You basically have a table arg that is a list of the arguments and arg.n is the length of the list. You can use that together with unpack(table) to achieve most things. Or you could try to recode things away from ... entirely and instead use tables.

As an example, the following

    function AIO.Handle(name, handlername, ...)
        assert(name ~= nil, "#1 expected not nil")
        return AIO.Msg():Add(name, handlername, ...):Send()
    end

AIO.Handle("name", "handler", "some", "args", 123)

could be written with varargs as

    function AIO.Handle(name, handlername, ...)
        assert(name ~= nil, "#1 expected not nil")
        return AIO.Msg():Add(name, handlername, unpack(arg)):Send()
    end

AIO.Handle("name", "handler", "some", "args", 123)

or with tables as

    function AIO.Handle(args)
        assert(args[1]~= nil, "#1 expected not nil")
        return AIO.Msg():Add(args):Send()
    end

AIO.Handle({"name", "handler", "some", "args", 123})
-- Or perhaps even the following, which is syntactic sugar
AIO.Handle{"name", "handler", "some", "args", 123}
TheSkullbot commented 3 years ago

I managed to "fix" using your tips with unpack, but it seems the smallfolk lib has the same issues with LUA 5.0. The data seems to be serialized in the wrong way. Might be because of the # operator changed to either string.len() or table.getn()

Here's a Gist of the modified AIO.lua and smallfolk.lua so far.

AIO : https://gist.github.com/TheSkullbot/642f174b8c576091edf63a19339f4e94 Smallfolk : https://gist.github.com/TheSkullbot/a94d8997d17a482153a7da0feae0085d

EDIT : I managed to make the serialization work by tweaking the smallfolk lib for 5.0 (I updated the gists) Now, I need to find out how to send ADDON_MESSAGE with the WHISPER channel disabled. I will try to sent it through regular chat message maybe ?

Rochet2 commented 3 years ago

Any message type likely works ok. You can potentially hide messages from the chat window itself if they show up there. However, you must be able to have a check like this in place if sender == UnitName("player") then which is the reason why whisper was used originally. I am not sure if other message types also have the sender as a part of the event data. It is crucial for authenticity that the client knows that all messages it receives were made by the server, not another player. The original code uses the player itself as the whisper sender when the server sends a message, so the client can check that sender is equal to the player itself.

You may want to stick to the addon language though as that is not affected by some checks and is not affected by in game language, drunkenness etc modifiers.

TheSkullbot commented 3 years ago

The WHISPER type is supported for SendAddonMessage from version 2.1, so I may try to use the regular WHISPER chat message. I don't know yet if the whisper sent to a non existing player is received by the server (I assume it is, for it to check the receiver exists)

If it is, then we just need to check the LANG (not sure if we can send regular chat message in LANG_ADDON though) And yes, it prevents drunkeness modifications and special chars limitation

I'll keep you updated on the progress, and if a working solution is found :)

TheSkullbot commented 3 years ago

Ok, so I had to go through the BATTLEGROUND channel instead of the WHISPER channel for SendAddonMessage client side. The packet is sent to the server even if your not in a battleground (unlike PARTY or GUILD). I had to move the Eluna:OnChat before the battleground check server side (and make sure it's in LANG_ADDON)

Now, I'm receiving the addon code from the server ! But here's a new issue (still with KaevStats) !

2021-06-13 18_44_38-World of Warcraft

Line 719 being the one doing the concat of previous packages parts :

    -- Has all parts, process
    if table.getn(data.parts) == data.parts.n then
        local cat = tconcat(data.parts) -- Line 719
        RemoveData(guid, messageId)
        AIO_ParseBlocks(cat, player)
    end
Rochet2 commented 3 years ago

Try to print the contents and types of everything in data.parts. for k,v in pairs(data.parts) do print(k, v, type(v)) end

TheSkullbot commented 3 years ago

The issue was that data.parts.n got in the loop (containing the size of the table, thus a number, triggerring tconcat error) With this fixed, all 32x 244 char + 1x 192 char parts were bound together in a single 8000 chars string(see dump below). Smallfolk seems to have some issues unserializing it though.

2021-06-14 04_57_46-World Of Warcraft Screenshot 2021 06 14 - 04 57 40 96 png - XnView MP

So I tried with the basic Hello World example (with a tweak to display the message in the chat instead of print) No unserialize/smallfolk errors, but it doesn't work triggers anything.

2021-06-14 05_36_18-debian@SKL-ORNSTEIN_ _mnt_c_Dev_mythic-wow_mythic-core-classic_bin_x64_Release_L 2021-06-14 05_19_07-World of Warcraft

I stopped here for today : In _AIO_HandleIncomingMsg, the msgid is a 2 char string, apparently the two schar(1) meaning it's a AIO_ShortMsg. Somehow, AIO_HANDLERS.Init() is never called I'll investigate further tomorrow :)

sundayz commented 3 years ago

Nice research into the issue, I was surprised that the 1.12.1 client was using 5.0 instead of 5.1 It looks like a lot of work to make AIO_Client work with 1.12.1, that client is really starting to show its age

Niam5 commented 2 years ago

Ok, so I had to go through the BATTLEGROUND channel instead of the WHISPER channel for SendAddonMessage client side. The packet is sent to the server even if your not in a battleground (unlike PARTY or GUILD). I had to move the Eluna:OnChat before the battleground check server side (and make sure it's in LANG_ADDON)

Now, I'm receiving the addon code from the server ! But here's a new issue (still with KaevStats) !

2021-06-13 18_44_38-World of Warcraft

Line 719 being the one doing the concat of previous packages parts :

    -- Has all parts, process
    if table.getn(data.parts) == data.parts.n then
        local cat = tconcat(data.parts) -- Line 719
        RemoveData(guid, messageId)
        AIO_ParseBlocks(cat, player)
    end

Can you submit what you did to the CMangos repo that I maintain. You can open up a new issue or a pull request.