otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.57k stars 1.05k forks source link

Chat channel onJoin message gets send to all others but not the one that joined #4618

Open xmish opened 8 months ago

xmish commented 8 months ago

https://github.com/otland/forgottenserver/blob/6579f1263271b0c431b00ae779b5b102569178d8/data/chatchannels/scripts/advertising.lua#L42C10-L45

onJoin message gets sent to all other members but the one that joined as it uses sendToAll https://github.com/otland/forgottenserver/blob/6579f1263271b0c431b00ae779b5b102569178d8/src/chat.cpp#L117C6-L117C17

client

xmish commented 8 months ago

my naive workaround (delayed as OTClient complained that channel is not yet open (channel is null - problem in OTClient I guess)):

    addEvent(function(pid) 
        local player = Player(pid)
        if not player then
            return
        end

        player:sendChannelMessage("", "Here you can advertise all kinds of things. Among others, you can trade items, advertise ingame events, seek characters for a quest or a hunting group, find members for your guild or look for somebody to help you with something.", MESSAGE_GUILD, CHANNEL_ADVERTISING)
        player:sendChannelMessage("", "It goes without saying that all advertisements must conform to the Rules, e.g. it is illegal to advertise trades including real money.", MESSAGE_GUILD, CHANNEL_ADVERTISING)
    end, 100, player:getId())
gesior commented 2 months ago

onJoin event is used to check, if player is able to "join" given channel (return false blocks access to channel). Not to send message to all players on given channel (including new channel member). addEvent is some work-around and I think it's acceptable - you can set timeout to 0 (not 100), it just has to execute after you return true from onJoin event. Maybe another event onJoined should be executed after joining channel and onJoin should be named to canJoin.

I don't think it's a bug. onXxx convention is used on OTSes for 15 years. It often refers to canXxx (onEquip = canEquip etc.). In case of onEquip is was changed on TFS 1.0+ - some version (1.2+ ?) - to extra parameter isCheck: executeEquip It's executed one time with false to check, if item can be equipped (return false blocks equip action) and then second time with true after item is equipped (return is ignored).

If someone plans to fix it, please use isCheck (canJoin = true) convention (extra Lua parameter) for old TFS compatibility. return false from onJoin should block access to channel.

nekiro commented 1 month ago

Not a bug, you send the message when player that is currently joining channel isn't yet there, so he cannot see it.

xmish commented 1 month ago

@nekiro i noticed that bebavior, but sending that message is not my code. It’s in tfs master thus i noticed it being sent and thought either it worked differently before or it was written on assumption that this works differently