Trinitycore: Logging does not display an item

[sernels]

Description:

Set worldserver.conf
Appender.GM=2,3,15,gm.log
Logger.commands.gm=3,Console GM

Expected behaviour:

CHANGEME Tell us what should happen instead.

Steps to reproduce the problem:

1. go to server .add [link items]
2. go to gm.log_date
3. see 2021-04-24_14:47:03 INFO [commands.gm] Command: add |cffffffff [Player: pulka (GUID Full: 0x0000000000000014 Type: Player Low: 20) (Account: 1) X: -8835.155273 Y: 629.580811 Z: 94.033585 Map: 0 (Eastern Kingdoms) Area: 1519 (Stormwind) Zone: Unknown Selected: (GUID Full: 0x0000000000000000 Type: None Low: 0)]

And you see add |cffffffff.
Branch(es):

3.3.5

TC rev. hash/commit:

https://github.com/TrinityCore/TrinityCore/commit/8db2ef9cdd415ed0d4897ec4d664ddba358cbaa1

Operating system:

Ubuntu 20.04

[sernels]

Sum @Treeston )))

[Treeston]

Remind me Friday next week if I haven't taken further action, I'm a bit swamped right now.

[Jildor]

seems happens with all hyperlinks inside commands, I think the problem is for here: https://github.com/TrinityCore/TrinityCore/blob/a32b6b8ac4c81128a56b282f20837fb085bdd9e7/src/server/game/Chat/ChatCommands/ChatCommand.cpp#L294

I am not familiarized with this part of the code, this may fix the problem, but it is probably not a suitable solution:

diff --git a/src/server/game/Chat/ChatCommands/ChatCommand.cpp b/src/server/game/Chat/ChatCommands/ChatCommand.cpp
--- a/src/server/game/Chat/ChatCommands/ChatCommand.cpp
+++ b/src/server/game/Chat/ChatCommands/ChatCommand.cpp
@@ namespace Trinity::Impl::ChatCommands
     if (cmd)
     { /* if we matched a command at some point, invoke it */
         handler.SetSentErrorMessage(false);
+        std::string fullCmdStr = std::string(cmdStr);
         if (cmd->IsInvokerVisible(handler) && cmd->_invoker(&handler, oldTail))
         { /* invocation succeeded, log this */
             if (!handler.IsConsole())
-                LogCommandUsage(*handler.GetSession(), cmd->_permission.RequiredPermission, cmdStr);
+                LogCommandUsage(*handler.GetSession(), cmd->_permission.RequiredPermission, fullCmdStr);
         }
         else if (!handler.HasSentErrorMessage())
         { /* invocation failed, we should show usage */

[Treeston]

I don't see how that changes anything, @Jildor; you're just converting string_view to string, and then implicitly back since LogCommandUsage takes string_view.

[Jildor]

because when executes cmd->_invoker(&handler, oldTail) you loose part of string_view cmdStr, I tested and debugged and works, but as I said I am not familiarized with this part of the code, so I'm not 100% sure what I do
I put it in case it helps;)

[Treeston]

Hmm, does .add's script modify the passed string_view's contents? It really shouldn't do that....

[Shauren]

.add uses legacy handler, mangling the input string with strtok

So I was thinking this would be a better targeted fix

diff --git a/src/server/game/Chat/ChatCommands/ChatCommand.h b/src/server/game/Chat/ChatCommands/ChatCommand.h
index b1c9896909..32f392f5dd 100644
--- a/src/server/game/Chat/ChatCommands/ChatCommand.h
+++ b/src/server/game/Chat/ChatCommands/ChatCommand.h
@@ -147,7 +147,10 @@ namespace Trinity::Impl::ChatCommands
         {
             _wrapper = [](void* handler, ChatHandler* chatHandler, std::string_view argsStr)
             {
-                return reinterpret_cast<bool(*)(ChatHandler*, char const*)>(handler)(chatHandler, argsStr.empty() ? "" : argsStr.data());
+                // make a copy of the argument string
+                // legacy handlers can destroy input strings with strtok
+                std::string argsStrCopy(argsStr);
+                return reinterpret_cast<bool(*)(ChatHandler*, char const*)>(handler)(chatHandler, argsStrCopy.c_str());
             };
             _handler = reinterpret_cast<void*>(handler);
         }

note: completely untested

[Treeston]

That's the handler that is only used for legacy char const* handlers, right? If yes, that fix is fine by me, since we can just remove it once I (or someone else) finds time to finish porting the last few files to the new system.

[Shauren]

yes, thats the (ChatHandler*, char const*) one

[Treeston]

LGTM then

[github-actions[bot]]

ab1a5b7fc8fca3b6540a49c39fc00cb63f16dc6a

[Aokromes]

the last few files to the new system.

and those are?

[Treeston]

Find-in-files the pragma that's used to suppress deprecation, all of the unported files will have that at the start. I don't remember which exact files are missing.