[Freeciv-Dev] (PR#13262) RFC: pubserver-in-a-diff

["Reinier Post" <rp@xxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13262 >

> [per - Mon Jun 13 19:49:37 2005]: 
>  
> This patch implements the ./configure switch --enable-pubserver. When 
you 
> use it: 
>  * You cannot manually save games (for security reasons) 
>  * /write is disabled (for security reasons) 
>  * Whenever a new server session is started (from starting the server or 
>    restarting it), a new output directory is created at the --saves DIR 
>    address + a unique number (eg civserver --saves /tmp may give 
/tmp/12 
>    as directory). All savegames, cmdline output and gamelogs are put 
>    there. 
>  * The 'load' command is ALLOW_CTRL (not hack) and takes a game 
number, 
>    not a filename. 
>  * Some default settings are different, notably quitidle and saveturns 
>  * The savename setting is disabled, all savegames are called 
>    'civgame.sav', and only one savegame is created for each game 
session. 
>  
> TODO 
>  * Make rulesetdir and /read work too. 
>  
> DISCUSS: 
>  * Jason wants it to be a cmdline option instead. This is a bit hard the 
>    way it is currently done, which you will see from the patch. 
>  * Perhaps an automated game session reaper to clean out old sessions 
on 
>    server start/restart should be written. 
>  * A way to compile in an admin password? 
>  * More savegames? 
>  
>   - Per 
 
I appreciate your effort, but all I can think is: UGH.  You are implementing 
the wrong way of achieving functionality that is already implemented the 
right way.  Why not evolve the right way of doing things instead of adding 
new, architecturally wrong methods of doing it next to the existing 
mechanisms, that will become lame and dysfunctional as a result? 
 
The beauty of pubserver, in my view, has always been that we had a 
really neat architecture: separation of concerns was properly dealt with by 
putting each concern into a different executable. 
 
One proof of that was that we didn't even have to make any changes to the 
civserver executable: civserver hardly needed to know whether it was 
operating on a pubserver or not, the system around it took care of that. The 
only thing that was added was the cmdlevel system.  The cmdlevel system 
is clean because it is a generally useful mechanism for client-side access 
level control.  It represented the exact bit of knowledge we needed to add 
to civserver in order to be able to operate it on a pubserver, but that 
knowledge was cleanly factored out. and generally available.  All the rest 
of the pubserver functionality was carefully kept out of civserver.  I admit I 
am quite a purist in this regard; I was against the --quitidle patch, for 
example, because we already had that functionality implemented in the 
wrapper script, and civservers don't generally need it. 
 
This patch goes completely against that. idea  It doesn't separate out the 
minimum knowledge that civserver needs in order to run on pubserver and 
add that to civserver, but instead, takes a bunch of things you need without 
a clear architectural motivation, and turns them a compile time option that 
more or less haphazardly changes civserver behavior.  AARGH!  Most of 
the functionality in this patch wasn't added to civserver on purpose, 
because it would spoil the architecture; the rest wasadded in the form of the 
cmdlevel system.  If the cmdlevel system is no longer good enough to 
capture the functionality required in civserver to support running on 
pubserver, either improve it or take it out, but please don't add a more or 
less randomly designed bit of other code to civserver that replicates its 
functionality while crashing through the original architecture, because it 
cripples and corrupts the cmdlevel system.that is already there.  The 
cmdlevels just don't mean wghat they were designed to mean anymore. 
 
Or, to put it another way, this patch is a great incentive to have a good 
discussion on the pubserver architecture, and that discussion was long 
overdue, partly because I documented things so poorly - but I don't think 
including the patch in any form is the right solution.