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 chat_log_level setting #9223

Merged
merged 3 commits into from May 14, 2020
Merged

Conversation

SmallJoker
Copy link
Member

@SmallJoker SmallJoker commented Dec 19, 2019

Closes #9199.

  • New setting chat_log_level to log to the chat
  • Colored similar to the terminal output (color codes somewhat trivial, debatable)
  • log.h cleanup -> move to source file for compiling time

Preview

To do

This PR is Ready for Review.

How to test

  1. Heat your room by recompiling the entire Minetest source code
  2. minetest.conf:
chat_log_level = info
  1. Get spammed.

@SmallJoker SmallJoker added the Feature ✨ PRs that add or enhance a feature label Dec 19, 2019
{
bool colored_message = (Logger::color_mode == LOG_COLOR_ALWAYS) ||
(Logger::color_mode == LOG_COLOR_AUTO && is_tty);
if (colored_message) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No functional changes. Added braces here according to the code style guidelines.

@SmallJoker
Copy link
Member Author

Pinging @Wuzzy2 because he requested this feature.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Dec 22, 2019

Thanks, I have tested this for a bit now. So, one problem I found is that Minetest does not guarantee the log message actually appears in chat if that log message appears early at startup time.

I can reliably reproduce this with the following steps:

  1. Create a dummy mod with a single line: minetest.log("warning", "THIS IS A TEST WARNING!")
  2. Set chat_log_level to warning
  3. (Re-)Start Minetest
  4. Enable the dummy mod
  5. Start Minimal Development Test

The warning should be visible in chat. But it won't be visible. When I restart Minimal Development Test (WITHOUT restarting Minetest!), the warning will then reliably appear on every subsequent restart.
Apparently the log message fails to appear at the startup of game if you started up a game for the first time in the current Minetest session. At least that's how it looks to me. There might something else be going on as well. If you restart Minetest, you will get the same results.

Now some random nitpicks:

Like "Debug log level" but shows log messages in the chat.

Restart Minetest to apply changes.

I don't like descriptions that are written in the form “it works like setting XYZ”. Better write a full standalone description instead, even if it's a little redundant. Why? Because in Minetest, the user will only see one setting description at a time, and it's just inconvenient when you have to hunt down the other setting just to find out what chat_log_level does.

There's a leading space before the tab in line 400 of src/log.cpp.

Log all higher levels in LogOutputBuffer
Move StreamLogOutput::logRaw to source file like LogOutputBuffer::logRaw for compiling speed
@SmallJoker
Copy link
Member Author

@Wuzzy2 Thanks for testing. I fixed your reported issues in d8119d5.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented May 2, 2020

Works fine for me now.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works.

@SmallJoker SmallJoker merged commit 836dd4a into minetest:master May 14, 2020
@SmallJoker SmallJoker deleted the chat_log_setting branch July 5, 2020 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to display more debug log output in chat
3 participants