vloup / sgfork

Automatically exported from code.google.com/p/sgfork
GNU General Public License v2.0
0 stars 1 forks source link

Server map cycle #2

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Description:
@clientConnect()
    // Joe Kari: exec content of the onEvent_playerConnect_do cvar
    if ( firstTime && !isBot )
    {
        trap_SendConsoleCommand( EXEC_APPEND, "vstr 
onEvent_playerConnect_do\n" ) ;
        trap_SendConsoleCommand( EXEC_APPEND , va( "vstr 
onEvent_playerUpTo%i_do\n" , g_humancount ) ) ;
    }

Also a lot of redundant variables:
global g_humancount
humancount @ CalculateRanks()
CalculateRanks isn't the place to count "humans"

There is no point to check if it is bot or not since the variable is made 
for playercounting (including bots!). Maybe there is a reason not to count 
specs, but not bots since they are playing. And such variable is should be 
available for game module.
And moreover whole this thing is looks like dirty hack. Better to figure 
out other solution for a server to choose maps with dependence on players 
count. Maybe to store a players count or command which server may execute 
in its string.
Also such method isn't a secure one.

Original issue reported on code.google.com by igorpana...@gmail.com on 19 Aug 2009 at 6:16

GoogleCodeExporter commented 9 years ago
Yes, that isn't really secured since onEvent_playerConnect_do is executed by the
server binary (without qvm restriction) and could have been modified by 
corrupted qvm.

About the redundant variable, I'm sure it's better to count humans at the 
beginning
of the game and increase/decrease the value every time people 
connects/disconnects.

Original comment by kikc...@gmail.com on 21 Aug 2009 at 8:30

GoogleCodeExporter commented 9 years ago
Security hole is removed in r38.
Issue is renamed.

Original comment by igorpana...@gmail.com on 24 Aug 2009 at 8:27

GoogleCodeExporter commented 9 years ago
Hum, I'm perplexed... in that way, should we consider "nextmap" cvar to be 
insecure ?
Can you explain us how "nextmap" differs from "onEvent_*" cvars in the 
execution way so ?

Maybe just the human count should be recoded in a more sexy way.

Téquila

Original comment by bougar...@gmail.com on 7 Sep 2009 at 10:50

GoogleCodeExporter commented 9 years ago
nextmap is internal CVar that is used to determine what will happen after map 
is over.
Yes, it is pretty insecure too. Tremulous has a nice way to solve the trouble 
with 
nextmap, but it is a separate issue for sure (and probably Tremulous code isn't 
the 
best way to do it).
Concerning current issue - the removed code wasn't secure and was dirty hack.
There is no accepted solution at the moment.

Original comment by igorpana...@gmail.com on 7 Sep 2009 at 1:27

GoogleCodeExporter commented 9 years ago
I propose the following solution:
The server (I think binary - SV, not GAME) has the variable:
sv_maplist <maplistfile name>
This maplist file is read to server's array maplist which consists of the 
following 
parameters for each map:
mapname: dm_*
gametype: 1
minplayersToStart: -
maxplayersToStart: 8
timelimit: -
fraglimit: -
etc.. All parameters are to be discussed.

This file is read when server starts. Each time when current match is over the 
next 
map from the list is called. If parameters of current game allow to start next 
map 
(min/max players, etc) then it is started, otherwise it is skipped and next map 
is 
going to be checked.

The file is consts of fields to read and parse:
<mapname> <gametype> <maxplayers> <minplayers> etc

nextmap command is just command without parameters to force change to next map. 
It is 
not a CVar which will be executed.

Not sure about Tremulous code, but I think they are using something similar.

Original comment by igorpana...@gmail.com on 16 Sep 2009 at 5:27