Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add /say chat-command #8021

Open
wants to merge 1 commit into
base: master
from

Conversation

@ClobberXD
Copy link
Contributor

commented Dec 27, 2018

It feels hacky, not to mention it's kinda annoying, to add a space before a message if it starts with / to not be parsed as a chat-command. Presenting /say, the chat-command that displays the raw message as-is without checking if it's a chat-command. Many popular IRC daemons and clients provide a SAY command for the same reason.

Example

<PlayerA> How to PM you?
(PlayerB types "/say /msg PlayerB message")
<PlayerB> /msg PlayerB message

The short description (params) and long description (description) could be improved. Any suggestions?

@nerzhul

This comment has been minimized.

Copy link
Member

commented Dec 27, 2018

sorry but it's not compatible with CSM or chat protocol, you should be compliant with it, if somebody customize the player chat

@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Dec 27, 2018

Alright, what can I do to make it compliant with the chat protocol?

@ghost

This comment has been minimized.

Copy link

commented Dec 28, 2018

Another use case is the Command Block node from Mesecons. It can wait for 5.1 though.

As for the chat protocol thing, it seems to me that if there's an API, it's not exposed to Lua, and that whatever the problem is, the /me command from builtin has the same problem.

@HybridDog

This comment has been minimized.

Copy link
Contributor

commented Jan 6, 2019

Alright, what can I do to make it compliant with the chat protocol?

The function which recognizes chat commands is there:

core.register_on_chat_message(function(name, message)
if message:sub(1,1) ~= "/" then
return
end

You could add a global boolean which can be used to temporarily disable chat commands, for example

core.chatcommands_enabled = true
core.register_on_chat_message(function(name, message)
	if message:sub(1,1) ~= "/"
	or not core.chatcommands_enabled then
		return
	end

Then in the say chatcommand, set core.chatcommands_enabled to false, send the chat message and then set core.chatcommands_enabled to true. This blocks chatcommand testing for this one message.

To send the chat message, firstly execute all functions in minetest.registered_on_chat_messages, as done at https://github.com/red-001/more_chatcommands/blob/cfad0ec62a93e686cb8b4711ff5a1df30126dc90/init.lua#L6-L10.
Then, if none of these functions returned true, you can send the message with core.chat_send_all("<" .. name .. "> " .. param)
https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L3666-L3669

I'm not sure if all functions in registered_on_chat_messages need to be executed or it should abort when one function returns true. I'm also not sure if sending the message with chat_send_all always does what it should do.

@ghost

This comment has been minimized.

Copy link

commented Jan 6, 2019

Executing commands on /say defeats its whole purpose, doesn't it?

Edit: Ah, I read it wrong.

@ghost

This comment has been minimized.

Copy link

commented Jan 6, 2019

In fact, the Mesecons commandblock already registers this very command, except it needs server privileges instead of shout (I guess that's to prevent having an automatic block that spams the chat).

https://github.com/minetest-mods/mesecons/blob/737f366741f54659b17bd9c96e2232eedb9735ee/mesecons_commandblock/init.lua#L1-L8

@DS-Minetest

This comment has been minimized.

Copy link
Contributor

commented Jan 6, 2019

Executing commands on /say defeats its whole purpose, doesn't it?

This is why it was suggested to use a global boolean.

To send the chat message, firstly execute all functions in minetest.registered_on_chat_messages, as done at https://github.com/red-001/more_chatcommands/blob/cfad0ec62a93e686cb8b4711ff5a1df30126dc90/init.lua#L6-L10.

Simply use core.run_callbacks(core.registered_on_chat_messages, 5, name, message) as here.

@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

@HybridDog

... in the say chatcommand, set core.chatcommands_enabled to false, send the chat message and then set core.chatcommands_enabled to true. This blocks chatcommand testing for this one message.

To send the chat message, firstly execute all functions in minetest.registered_on_chat_messages ...
Then, if none of these functions returned true, you can send the message with core.chat_send_all("<" .. name .. "> " .. param)

I don't understand why chat-command testing should be disabled here. /say is a proper chat-command that just passes the message to chat_send_all. The message won't be passed to chat_send_all when typed directly, if the first char is / or ., to prevent which, one can just /say the exact same message. The on_chatmessage callback in builtin that displays a normal message in the typical <sender> msg format won't display this, because this is a chat-command. As far as the engine is concerned, this is a proper, legit chat-command. Only the command handler itself knows that it'll just echo the passed message in the format.

But I think I understand the problem now. This command hard-codes the output format to be <name> msg and this could lead to incompatibilities when the builtin chat message echoing callback changes the output format. If that's the case, I could make that callback a global function (similar to core.send_join_message), that is passed to core.register_on_chatmessage.

@HybridDog

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2019

Executing the registered_on_chat_messages is done that things which happen to usual chat messages (except chatcommand execution) are also applied to the message passed with /say, for example chat message colourization if some mod implemented it.
A global function where you can pass a name and message to do the same as what happens when a real chat message is received from a player could be helpful, not only for this PR.

@paramat paramat added the WIP label Apr 4, 2019
@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2019

Chat message format is hard-coded as I suspected:

minetest/src/server.cpp

Lines 2915 to 2918 in e32a630

line += L"<";
line += wname;
line += L"> ";
line += wmessage;

What's to be done here is to first make this a Lua API method, or to at least allow overriding this format using an API method.

@SmallJoker

This comment has been minimized.

Copy link
Member

commented May 12, 2019

What's to be done here is to first make this a Lua API method, or to at least allow overriding this format using an API method.

@ClobberXD So you mean something else than registering a on_chat_message callback?

@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@SmallJoker Yes. I'm thinking of implementing an API method that sends takes two params name and message, that calls Server::handleChat internally, which would format the chat message according to chat_message_format in minetest.conf (TODO). While mods can already send raw chat messages using chat_send_all, this can be used by mods to send a "player" chat message (e.g. message). This is required for /say to work, but it also has many other usecases - mods will have to use this API method instead of doing, say, minetest.chat_send_all("<" .. name .. "> " .. message). I plan on implementing customizable chat message format and the new API method in a different PR.

@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

Customizable chat message format PR: #8529

@rubenwardy rubenwardy force-pushed the minetest:master branch 2 times, most recently from 0e3b135 to 39c54e1 Jun 21, 2019
@SmallJoker

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Workaround added. Please reopen if this is still needed (judging from "Possible close").

@SmallJoker SmallJoker closed this Sep 4, 2019
@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

@SmallJoker #8529 just makes implementing this possible in a non-hardcoded way, and in no way provides an alternative for the functionality proposed here :)

@SmallJoker SmallJoker reopened this Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.