The dedicated server console-say bug, how do we fix it?

[commie]

The console-say bug

I have been tracking down the bug Gig mentioned here: http://openarena.ws/board/index.php?topic=5344.msg54985#msg54985

Functions involved in the bug

This bug is caused by flaws both in the gamecode and engine.

There are 2 different functions that gets used when a user enters a say-command in console:

• SV_ConSay_f (in engine/code/server/sv_ccmd.c
• Svcmd_MessageWrapper (in gamecode/tt/game/g_svcmds_ext.c)

The engine-version works by sending a chat-command like chat "console: " + message to the gamecode.

The gamecode-version tries to use the G_Say-function (in gamecode/tt/game/g_cmds.c) to send the message. In the ent-parameter that specifies the sender it passes NULL.

G_Say doesn't check for NULL. So when it does ent->client->pers.netname to get the name of the sender of the message it follows an invalid pointer which points to garbage memory. Under normal circumstances this would lead to a crash but instead it reads a random string from memory (the string "own weapon]" in Gigs case). It probably doesn't cause a crash because the code is running in QVM. 

The engine-version will always get used instead of the gamecode-version if it is available.

How does the bug happen?

The function SV_AddOperatorCommands (in engine/code/server/sv_ccmd.c) only registers operator commands once during game initialization. It registers the commands say and tell if the game is running in dedicated-mode.

If the game starts in dedicated-mode then these commands get registered. If the game starts without dedicated mode the commands does not get registered.

If the player starts the game without dedicated-mode and later switches to dedicated-mode the say and tell commands have not been registered. In that case the broken gamecode-version of say is used when the admin issues a say-command from the console and the tell command does not work at all from console.

If the player starts the game with dedicated-mode and later switches to non-dedicated-mode the say and tell commands doesn't work  as they should console because the dedicated console commands are used instead of the normal player commands.

2 different approaches to solve the bug

The bug is a result of a bad interaction between the engine and the gamecode where both parts of the code have bugs.

The code for dedicated server console-chat is quite messy because some commands are in the engine, other commands are in the gamecode, and the say-command is in both, in a seemingly random way.

It looks like this:

Engine: say, tell
Gamecode: say, say_team

This also gives us multiple approaches to solve this.

1. Moving say and tell commands to gamecode

I find that the most cleanest way would be to move all dedicated console chat commands to gamecode. This is because the gamecode already have a boolean value associated which each command that flags if it can only be run from dedicated-mode or not.

This change would mean that we:

- Remove dedicated-console say and tell commands from engine-code
- Fix the dedicated-console say command in gamecode
- Add dedicated-console tell command in gamecode

If this method gets implemented all console-chat commands are in gamecode which is most logical.

The obvious disadvantage of this would be that older gamecode versions would still have the bug. It also makes the bug more noticable if hosting a server with older version of the gamecode (as then the broken say-command in gamecode will always be used).

2. Fixing SV_AddOperatorCommands

Another solution would be to change SV_AddOperatorCommands so that it updates the operator commands when the user switches to or from dedicated-mode.

To my knowledge changes this would mean is:

- Make so SV_AddOperatorCommands gets called at each new map instead of only at game-initialization (or each new map when the cvar dedicated has been modified)
- Remove broken say-command from gamecode

But it may require further changes at other parts of the code.

What solution do you think we should use?

I am ready to spend time trying to solving this.

But I would like your input on which approach should be used to fix it.

Should we go with the second option to also fix the servers which are running older gamecode?

Or should we go with the first option because of the added consistency in the system?

Do you have any other approach that might be better than the ones listed here?