Send on channel refactoring (#85)

* Refactor send_on_channel

* Fix all the mistakes

* Change how things work

* no_unused_args

* Prevent double formatting and move formatting further down in event sequence

* Fix wrong variable name

* Fix channel message before_send events

* Keep original message through send loop

common.lua

@@ -82,13 +82,13 @@ beerchat.is_player_subscribed_to_channel = function(name, channel)
 		and (nil ~= beerchat.playersChannels[name][channel])
 end
 
-beerchat.send_message = function(name, message, channel)
-	if not beerchat.execute_callbacks('before_send', name, message, channel) then
-		return
+beerchat.send_message = function(name, message, data)
+	if type(data) == "table" and beerchat.execute_callbacks('before_send', name, message, data) then
+		minetest.chat_send_player(name, data.message)
+	elseif beerchat.execute_callbacks('before_send', name, message) then
+		minetest.chat_send_player(name, message)
 	end
-
-	minetest.chat_send_player(name, message)
-	--[[ TODO: read player settings for channel sounds
+	--[[ TODO: read player settings for channel sounds, also move this from core to some sound effect extension.
 	if beerchat.enable_sounds and channel ~= beerchat.main_channel_name then
 		minetest.sound_play(
 			beerchat.channel_message_sound, {

hooks.lua

@@ -1,44 +1,39 @@
 
 beerchat.cb = {} -- all custom callbacks
 
-beerchat.cb.before_send 		= {} -- executed before sending message
-beerchat.cb.before_send_pm 		= {} -- executed before sending private message
-beerchat.cb.before_send_me		= {} -- executed before /me message is sent
-beerchat.cb.before_whisper		= {} -- executed before whisper message is sent
-beerchat.cb.before_join 		= {} -- executed before channel is joined
-beerchat.cb.before_leave 		= {} -- executed before channel is leaved
-beerchat.cb.before_switch_chan 	= {} -- executed before channel is changed
-beerchat.cb.before_invite 		= {} -- excuted before channel invitation takes place
-beerchat.cb.before_mute 		= {} -- executed before player is muted
-beerchat.cb.before_check_muted 	= {} -- executed before has_player_muted_player checks
-beerchat.cb.on_forced_join 		= {} -- executed right after player is forced to channel
+beerchat.cb.before_send        = {} -- executed before sending message
+beerchat.cb.before_send_pm     = {} -- executed before sending private message
+beerchat.cb.before_send_me     = {} -- executed before /me message is sent
+beerchat.cb.before_whisper     = {} -- executed before whisper message is sent
+beerchat.cb.before_join        = {} -- executed before channel is joined
+beerchat.cb.before_leave       = {} -- executed before channel is leaved
+beerchat.cb.before_switch_chan = {} -- executed before channel is changed
+beerchat.cb.before_invite      = {} -- excuted before channel invitation takes place
+beerchat.cb.before_mute        = {} -- executed before player is muted
+beerchat.cb.before_check_muted = {} -- executed before has_player_muted_player checks
+beerchat.cb.on_forced_join     = {} -- executed right after player is forced to channel
 
 -- Special events
-beerchat.cb.after_joinplayer	= {} -- executed after player has joined and configurations loaded
+beerchat.cb.after_joinplayer   = {} -- executed after player has joined and configurations loaded
 
 -- Callbacks that can edit message contents
-beerchat.cb.on_receive 			= {} -- executed when new message is received
-beerchat.cb.on_http_receive 		= {} -- executed when new message is received through http polling
-beerchat.cb.on_send_on_channel		= {} -- executed before sending message to channel
+beerchat.cb.on_receive             = {} -- executed when new message is received
+beerchat.cb.on_http_receive        = {} -- executed when new message is received through http polling
+beerchat.cb.on_send_on_channel     = {} -- executed before delivering message to individual channel subscriber
+beerchat.cb.before_send_on_channel = {} -- executed before sending message to channel
 
 beerchat.register_callback = function(trigger, fn)
-	if type(fn) ~= 'function' then
-		print('Error: Invalid fn argument for beerchat.register_callback, must be function. Got ' .. type(fn))
-		return
-	end
-	if type(trigger) ~= 'string' then
-		print('Error: Invalid trigger argument for beerchat.register_callback, must be string. Got ' .. type(trigger))
-		return
-	end
-
+	assert(type(trigger) == 'string',
+		'Error: Invalid trigger argument for beerchat.register_callback, must be string. Got ' .. type(trigger))
+	assert(type(fn) == 'function',
+		'Error: Invalid fn argument for beerchat.register_callback, must be function. Got ' .. type(fn))
 	if not beerchat.cb[trigger] then
-		print(string.format('Error: Invalid callback trigger event %s, possible triggers:', trigger))
+		local err = {('Error: Invalid callback trigger event %s, possible triggers:'):format(trigger)}
 		for k,_ in pairs(beerchat.cb) do
-			print(' ->   ' .. k)
+			table.insert(err, ' ->   ' .. k)
 		end
-		return
+		error(table.concat(err, "\n"))
 	end
-
 	table.insert(beerchat.cb[trigger], fn)
 end
 

message.lua

@@ -15,32 +15,33 @@
 -- ${time} the current time in 24 hour format, as returned from os.date("%X")
 --
 
-beerchat.send_on_channel = function(name, channel_name, message)
-	minetest.log("action", "[beerchat] CHAT #" .. channel_name .. " <" .. name .. "> " .. message)
-	local msg = {name=name, channel=channel_name,message=message}
+local send_on_local_channel = function(msg)
+	local message = msg.message
 	for _,player in ipairs(minetest.get_connected_players()) do
 		local target = player:get_player_name()
-		-- Checking if the target is in this channel
-		if beerchat.execute_callbacks('on_send_on_channel', msg, target) then
-			beerchat.send_message(
-				target,
-				beerchat.format_message(
-					beerchat.main_channel_message_string, {
-						channel_name = msg.channel,
-						to_player = target,
-						from_player = msg.name,
-						message = msg.message
-					}
-				),
-				msg.channel
-			)
+		if beerchat.execute_callbacks('on_send_on_channel', msg.name, msg, target) then
+			beerchat.send_message(target, message, msg)
 		end
 	end
 end
 
-beerchat.register_callback("on_send_on_channel", function(msg, target)
-	if not beerchat.is_player_subscribed_to_channel(target, msg.channel)
-		or beerchat.has_player_muted_player(target, msg.name) then
-		return false
+beerchat.send_on_local_channel = function(msg)
+	if beerchat.execute_callbacks('before_send_on_channel', msg.name, msg) then
+		send_on_local_channel(msg)
 	end
-end)
+end
+
+beerchat.send_on_channel = function(msg, ...)
+	-- FIXME: Backwards compatibility hack. Remove once everything uses table for message handling.
+	if type(msg) ~= "table" then
+		local arg = {...}
+		msg = {name=msg, channel=arg[1], message=arg[2]}
+	end
+	-- Execute registered event handlers, abort if told to do so
+	if beerchat.execute_callbacks('before_send_on_channel', msg.name, msg) then
+		-- Log and deliver message to both local and remote platforms
+		minetest.log("action", "[beerchat] CHAT #" .. msg.channel .. " <" .. msg.name .. "> " .. msg.message)
+		beerchat.on_channel_message(msg.channel, msg.name, msg.message)
+		send_on_local_channel(msg)
+	end
+end

plugin/ban.lua

@@ -45,9 +45,9 @@ end
 -- Register beerchat event handlers for channel bans
 --
 
-beerchat.register_callback('before_send', function(name, message, channel)
-	if beerchat.ban.is_player_banned(channel, name) then
-		return false, "Sorry but you are banned on #"..channel..", you are not allowed to send messages there."
+beerchat.register_callback('before_send_on_channel', function(name, msg)
+	if beerchat.ban.is_player_banned(msg.channel, name) then
+		return false, "Sorry but you are banned on #"..msg.channel..", you are not allowed to send messages there."
 	end
 end)
 

plugin/colorize.lua

@@ -1,4 +1,4 @@
-
+--luacheck: no_unused_args
 --
 -- Allow using colors on chat messages by sending "(#f00)Red (#0f0)Green (#00f)Blue"
 --
@@ -16,8 +16,8 @@ if colorize_channels == "*" then
 
 	-- Colorize all chat channels
 
-	beerchat.register_callback('on_send_on_channel', function(msg_data)
-		msg_data.message = msg_data.message:gsub('%((%#%x%x%x)%)', string.char(0x1B) .. '(c@%1)')
+	beerchat.register_callback('before_send_on_channel', function(name, msg)
+		msg.message = msg.message:gsub('%((%#%x%x%x)%)', string.char(0x1B) .. '(c@%1)')
 	end)
 
 elseif colorize_channels then
@@ -31,9 +31,9 @@ elseif colorize_channels then
 	end
 
 	if next(allowed_channels) then
-		beerchat.register_callback('on_send_on_channel', function(msg_data)
-			if msg_data.channel and allowed_channels[msg_data.channel] then
-				msg_data.message = msg_data.message:gsub('%((%#%x%x%x)%)', string.char(0x1B) .. '(c@%1)')
+		beerchat.register_callback('before_send_on_channel', function(name, msg)
+			if msg.channel and allowed_channels[msg.channel] then
+				msg.message = msg.message:gsub('%((%#%x%x%x)%)', string.char(0x1B) .. '(c@%1)')
 			end
 		end)
 	end

plugin/event-logging.lua

@@ -14,7 +14,7 @@ beerchat.register_callback('on_forced_join', function(name, target, channel, fro
 		.. " from #" .. from_channel .. " to #" .. channel .. "."
 	if beerchat.moderator_channel_name then
 		local sender = beerchat.channels[beerchat.main_channel_name].owner
-		beerchat.send_on_channel(sender, beerchat.moderator_channel_name, move_msg)
+		beerchat.send_on_channel({name=sender, channel=beerchat.moderator_channel_name, message=move_msg})
 	end
 	-- inform admin
 	minetest.log("action", "CHAT " .. move_msg)

plugin/hash.lua

@@ -40,6 +40,28 @@ local function switch_channel(name, channel)
 	end
 end
 
+beerchat.register_callback("on_send_on_channel", function(name, msg, target)
+	-- Check subscriptions and muting, abort if not subscribed or target has muted sender.
+	if not beerchat.is_player_subscribed_to_channel(target, msg.channel)
+		or beerchat.has_player_muted_player(target, name) then
+		return false
+	end
+end)
+
+beerchat.register_callback("before_send", function(target, message, data)
+	if data and data.channel then
+		-- Apply formatting for channel messages.
+		data.message = beerchat.format_message(
+			beerchat.main_channel_message_string, {
+				channel_name = data.channel,
+				to_player = target,
+				from_player = data.name,
+				message = message
+			}
+		)
+	end
+end)
+
 beerchat.register_on_chat_message(function(name, message)
 	local channel_name, msg = string.match(message, "^#(%S+) ?(.*)")
 
@@ -55,8 +77,7 @@ beerchat.register_on_chat_message(function(name, message)
 	elseif not beerchat.is_player_subscribed_to_channel(name, channel_name) then
 		minetest.chat_send_player(name, "You need to join this channel in order to be able to send messages to it")
 	else
-		beerchat.on_channel_message(channel_name, name, msg)
-		beerchat.send_on_channel(name, channel_name, msg)
+		beerchat.send_on_channel({name=name, channel=channel_name, message=msg})
 	end
 
 	return true

plugin/jail.lua

@@ -147,23 +147,20 @@ beerchat.register_callback('before_leave', function(name, channel)
 	end
 end)
 
-beerchat.register_callback("on_send_on_channel", function(msg, target)
-	if msg.channel ~= beerchat.jail.channel_name and beerchat.is_player_jailed(msg.name) then
+beerchat.register_callback("before_send_on_channel", function(name, msg)
+	if msg.channel ~= beerchat.jail.channel_name and beerchat.is_player_jailed(name) then
 		-- redirect #channel messages sent by jailed players toward jail channel and reconstruct full command.
 		msg.channel = beerchat.jail.channel_name
 		msg.message = "#" .. msg.channel .. " " .. msg.message
-		if not beerchat.is_player_subscribed_to_channel(target, msg.channel) then
-			return false
-		end
 	end
 end)
 
-beerchat.register_callback('before_send', function(name, message, channel)
-	if beerchat.is_player_jailed(name) then
-		if channel == beerchat.jail.channel_name then
+beerchat.register_callback('before_send', function(target, message, data)
+	if data and beerchat.is_player_jailed(data.name) then
+		if data.channel == beerchat.jail.channel_name then
 			-- override default send method to mute pings for jailed users
 			-- but allow chatting without pings on jail channel
-			minetest.chat_send_player(name, message)
+			minetest.chat_send_player(target, message)
 		end
 		return false
 	end

plugin/password.lua

@@ -36,7 +36,7 @@ local function handle_player(name, last_login)
 	end
 end
 
-beerchat.register_callback('before_send', function(name)
+beerchat.register_callback('before_send_on_channel', function(name)
 	-- Only run checks if account is marked for notifications
 	if password_notify[name] then
 		if minetest.get_us_time() - password_notify[name] > 4000000 then

router.lua

@@ -8,22 +8,21 @@ function beerchat.register_on_chat_message(func)
 	table.insert(on_chat_message_handlers, func)
 end
 
-local function default_message_handler(name, message)
+local function default_message_handler(msg)
 	-- Do not allow players without shout priv to chat in channels
-	if not minetest.check_player_privs(name, "shout") then
+	if not minetest.check_player_privs(msg.name, "shout") then
 		return true
 	end
 
-	local channel = beerchat.get_player_channel(name)
-	if not channel then
-		beerchat.fix_player_channel(name, true)
-	elseif message == "" then
-		minetest.chat_send_player(name, "Please enter the message you would like to send to the channel")
-	elseif not beerchat.is_player_subscribed_to_channel(name, channel) then
-		minetest.chat_send_player(name, "You need to join this channel in order to be able to send messages to it")
+	msg.channel = beerchat.get_player_channel(msg.name)
+	if not msg.channel then
+		beerchat.fix_player_channel(msg.name, true)
+	elseif msg.message == "" then
+		minetest.chat_send_player(msg.name, "Please enter the message you would like to send to the channel")
+	elseif not beerchat.is_player_subscribed_to_channel(msg.name, msg.channel) then
+		minetest.chat_send_player(msg.name, "You need to join this channel in order to be able to send messages to it")
 	else
-		beerchat.on_channel_message(channel, name, message)
-		beerchat.send_on_channel(name, channel, message)
+		beerchat.send_on_channel(msg)
 	end
 	return true
 end
@@ -53,5 +52,5 @@ minetest.register_on_chat_message(function(name, message)
 	end
 
 	-- None of extensions handled current message, call through default message handler
-	return default_message_handler(msg.name, msg.message)
+	return default_message_handler(msg)
 end)

web/rx.lua

@@ -11,10 +11,10 @@ local function handle_data(data)
 
 	if data.event == "user_action" then
 		-- "/me" message, TODO: use format and helper in "plugin/me.lua"
-		beerchat.send_on_channel(data.username, data.gateway, data.text)
+		beerchat.send_on_local_channel({name=data.username, channel=data.gateway, message=data.text})
 	elseif data.event == "join_leave" then
 		-- join/leave message, from irc for example
-		beerchat.send_on_channel(data.username, data.gateway, data.text)
+		beerchat.send_on_local_channel({name=data.username, channel=data.gateway, message=data.text})
 	else
 		-- regular text
 		if string.sub(data.text, 1, 1) == "!" then
@@ -28,12 +28,11 @@ local function handle_data(data)
 			end
 		else
 			-- regular user message
-			beerchat.send_on_channel(data.username, data.gateway, data.text)
+			beerchat.send_on_local_channel({name=data.username, channel=data.gateway, message=data.text})
 		end
 	end
 end
 
-
 local function recv_loop()
 	http.fetch({
 		url = beerchat.url .. "/api/messages",