fix listen-server crash

[neogeographica]

This is a fix for issue #8. The comments on that issue describe the problem. This change fixes that issue and is also just generally correct in that we shouldn't be drawing scoreboards if we aren't even signed into the game yet.

However it would be cool if a Quake engine coding veteran could verify that this particular change is the best (least fragile) way of addressing the issue. I've chosen the "bail out at the last minute" approach, and it might be that a higher-level fix to the control flow would be better.

[neogeographica]

Spirit got some comments from Spike on IRC (thanks!); summarizing those here along with some more thoughts:

• This should work "in all cases except those that are specifically crafted to still crash it". Which makes me a little nervous so hopefully there's a better solution.
• Doubt about whether this would work for QuakeWorld protocol. I see some QW code in reQuiem, not sure if it's meant to support QW too?
• "Probably its easier to just not call SCR_UpdateScreen from Con_Print" + some comments about that call mainly being for debugging. This would be my preferred solution if we can be sure that it doesn't break some non-debugging functionality. The entire Con_Print function in reQuiem is way different from the WinQuake code so I'm a little cautious about just removing the SCR_UpdateScreen call. I should probably test removing that call to see if console text still gets printed at all without it, when a map isn't running.
• "Alternatively, just change those calls to Con_SafePrintf." (Referring here to the calls from CL_ParseServerInfo.) Might be a functional band-aid but that still kind of leaves the vulnerable sbar code dangling in the wind.

[neogeographica]

Looking at code for WinQuake and QuakeSpasm: they don't call SCR_UpdateScreen from within Con_Print, but they do from its wrapper function Con_Printf. And they do call Con_Printf from CL_ParseServerInfo, before allocating cl.scores. But they don't crash when starting a listen server. Why is that?

One of those calls in CL_ParseServerInfo is a Con_DPrintf, which wraps Con_SafePrintf, so no problem there.

The other Con_Printf calls before the cl.scores alloc only happen in error conditions. I do wonder a little about these... they're unusual (bad protocol and bad maxclients) but I'm sure that at least the bad protocol error would be hit occasionally, and presumably it didn't crash the client. Maybe there's something about the sbar code that bailed out early in those circumstances before getting to Sbar_SortFrags.

(I guess you're not going to ever get bad protocol or bad maxclients in the listen-server case, so maybe that's the saving grace.)

Also CL_ParseServerMessage will call Con_Printf "too early" if cl_shownet is 1, but that doesn't cause crashes in WinQuake or QuakeSpasm when starting a listen server.

Next thing for me to do might be to get QuakeSpasm in a debugger to see why its SCR_UpdateScreen callstack doesn't crash in similar circumstance as reQuiem's.

[neogeographica]

OK, the reason that other Quake engines don't crash is that they have this at the top of Sbar_Draw:

    if (scr_con_current == vid.height)
        return;     // console is full screen

I.e. if console is fullscreen we shouldn't be trying to draw a scoreboard. This is kind of only indirect protection from drawing the scoreboard before cl.scores is allocated, but it's the traditional approach.

reQuiem doesn't have that check. Instead, it has

    if (cls.state != ca_connected)
        return;

...which may be helpful in some situations, but not in this particular issue.

I'm going to close this PR and open a new one for a different fix.

[SpiritQuaddicted]

Just a quick comment:

Doubt about whether this would work for QuakeWorld protocol. I see some QW code in reQuiem, not sure if it's meant to support QW too?

Not that I know. It does have support for playing QW demos though if I recall correctly. Not sure if just qwd or also qwz(?) or even mvd.