azerothcore-wotlk
azerothcore-wotlk copied to clipboard
chore. Remove error warning on worldserver
Changes Proposed:
This PR proposes changes to:
- Error while configuring Appender GM in Logger commands.gm. Appender does not exist
- [x] worldserver.conf
Issues Addressed:
- Closes
SOURCE:
The changes have been validated through:
Tests Performed:
This PR has been:
- [x] Tested in-game by the author.
How to Test the Changes:
- Having commented on this line, I don't know if the logs are generated or not, because it depends on the options below, but in the worldserver at the top, a message appears with an error. Let it continue, but this message should not appear.
Known Issues and TODO List:
How to Test AzerothCore PRs
When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].
You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.
Comment out the logger instead. It is bad to have this appender open by default
Comment out the logger instead. It is bad to have this appender open by default
Could you tell me which file I should search in? Thank you.
worldserver.conf.dist https://github.com/azerothcore/azerothcore-wotlk/blob/b6ee4f49cfbfc907b91fab446de7d207a2bff8bc/src/server/apps/worldserver/worldserver.conf.dist#L670
worldserver.conf.dist https://github.com/azerothcore/azerothcore-wotlk/blob/b6ee4f49cfbfc907b91fab446de7d207a2bff8bc/src/server/apps/worldserver/worldserver.conf.dist#L670
Thanks.
This pull request is ready to be tested, although it is not urgent. It's just to avoid that message that usually appears at the top of the worldserver when powering on.
I just tested this PR.
It does indeed remove the error message on server start BUT it also disables all debug messages when GM commands are beeing used.
So right now I can't approve this PR, as imo it's just a workaround and does not fix the "real" issue causing the error message.
We close the pull request then. Thanks for the test. It was open for quite some time. I would prefer that later someone corrects the error thoroughly as you mention. Greetings.
Alright so I've taken another look at this after a discussion with @pangolp
The "root cause" for the error message is the commit 12456b54 by @Kitzunu.
In this commit he disabled the Appender.GM
without also disabling Logger.commands.gm
, which results in the observed error message.
As of the hint provided by @pangolp, it seems that these two options can only exist together.
Therefore this PR indeed seems valid to fix this issue, as the intend of @Kitzunu in 12456b54 seems to have been to disable GM command debug messages by default and this PR finalizes this intention by fixing the error message.
If the changes in the mentioned commit were made by accident, this PR should instead uncomment Appender.GM
to properly enable GM command debug messages and also get rid of the error.
In the end right now this is a lose-lose situation as no feedback on this was provided, now I took a look, maybe couldn't grasp the whole situation on the first attempt, the PR author is disappointed, deletes the PR and (again) in the long run we might lose another contributor.
Alright so I've taken another look at this after a discussion with @pangolp
The "root cause" for the error message is the commit 12456b5 by @Kitzunu.
In this commit he disabled the
Appender.GM
without also disablingLogger.commands.gm
, which results in the observed error message.As of the hint provided by @pangolp, it seems that these two options can only exist together.
Therefore this PR indeed seems valid to fix this issue, as the intend of @Kitzunu in 12456b5 seems to have been to disable GM command debug messages by default and this PR finalizes this intention by fixing the error message.
If the changes in the mentioned commit were made by accident, this PR should instead uncomment
Appender.GM
to properly enable GM command debug messages and also get rid of the error.In the end right now this is a lose-lose situation as no feedback on this was provided, now I took a look, maybe couldn't grasp the whole situation on the first attempt, the PR author is disappointed, deletes the PR and (again) in the long run we might lose another contributor.
It's not your fault that you may have interpreted things wrong, possibly I explained them wrong. Instead of closing it, I should have explained or changed the title as you suggested in the message, so don't worry. Thank you for having done the tests, and after having read my explanation, continue testing the comments I made about it.
I reopen the pull request then.
@sudlud In summary, according to your tests, should we then have both lines commented or just having this one is enough?
This PR finalizes the intent of commit 12456b5 to disable gm command dbg messages by default and also disables the error message on startup.
But right now the dbg messages are being printed out in worldserver console together with an error on server start.
So in the end we need the maintainers' decision now:
- should gm command dbg messages in worldserver console be enabled by default?
Yes - this PR can be merged as-is No - this PR needs an update
This PR finalizes the intent of commit 12456b5 to disable gm command dbg messages by default.
But right now the dbg messages are being printed out in worldserver console together with an error on server start.
So in the end we need the maintainers' decision now:
- should gm command dbg messages in worldserver console be enabled by default?
Yes - this PR can be merged as-is No - this PR needs an update
I would disable messages by default, since they are very annoying, and I would let each project decide whether it wants to enable them or not. At most, information can be added to the wiki so that each community can correctly configure the logs of its worldserver.conf and authserver.conf, because the vast majority of them are left by default.