I'm able to reproduce this problem. I'll see what's up when I have time. 

#1385
The "semi-official" 32-player deathmatch map death32c, still available on ID's FTP server (ftp://ftp.idsoftware.com/idstuff/quakeworld/maps/) has lightmapped water.
And the lightmaps are messed-up
[...]
other maps with messed-up lightmaps on water surfaces may very well be out there in the wild, but in the absence of engine support for lightmapped water nobody was ever aware of them.
[...]
Since lightmapped water is a new feature I suggest a worldspawn key indicating it's presence. That way engines that don't support it can ignore it. Engines that do support it can pick it up. Maps like death32c aren't subject to false positives.
#1389
DarkPlaces shows the same problem on death32c

Just because mh, lordhavoc and others don't know how to reliably detect lit water, it doesn't mean it's impossible. Retroquad never accepts false positives.

Cluttering the tools and engine code with a worldspawn key for this is pointless, and only serves to hide the fact that the engine's support for lit water is done poorly.

Also, let me rectify Baker's false statement @ #1285:
Mankrip said maps like e1m4 don't compile with lit water.
This is false. What I said is that a few ID1 GPL maps such as e1m4 has countless leaks. This means that their VIS data can't be compiled, which has nothing to do with the lighting. I've already compiled it with lit water and no VIS ages ago, but I wanted to be able to get the VIS data compiled too.

Until someone steps forward proving any of this stuff works and is polished ... I'm not sold.
The solution in Retroquad works flawlessly, is robust, clean and polished. But I try not to care about proving stuff for others. Nobody ever asked me about it, it would just make me seem bitter, and I'm sure I've already mentioned ages ago that the Retroquad implementation of lit water was finally perfect.

Also, <a href="https://twitter.com/mankrip/status/774418444523606016">by the end of last September, EricW's implementation in the light tool was already perfect too. I've done extensive testing for him and showed it when the final problems got fixed. 

 

I always have net_connecttimeout set to 45 seconds, and I know the players with the connection trouble aren't using any external textures (I keep telling them they should try the external textures, heh).

It could very well be some issue with the server. It's just odd how it only affects certain players, and it affects them repeatedly, but nobody else. Extra odd how it affects BOTH these two new players....

Though the fact that it's known that typing "ping" in the console can sometimes get around it, would indicate that it's been a Quake issue for a long time....

I can't really provide enough information about it to really troubleshoot :/ 

I didn't know the GPL map sources for e1m4 had leaks. I thought when you mentioned the problem that it was a tool issue when using lit water.

Anyway, the conversation evolved a lot since then and yeah ... I'm sold now.

But my non-belief was largely due to the apparent non-existence of any known test maps.

When avirox, myself and gb were working on true rotation, we made a number of test maps with QuakeC sources, engine sources, .map files and map compile instructions. 

I guess if one of them could play using "developer 1" and also record a demo, then the next time they get the problem zip up the demo + qconsole.log, there would be some information to go on.

From your description about how typing "ping" helps, it sounds like a message got blocked by a firewall in a non-NAT fixed client.

If I had a qconsole.log where developer 1 was enabled where one of these players had the problem, I might be able to learn more.

I just now connected to your server, I don't experience this problem. And apparently you don't.

At the moment, there really isn't enough information to speculate. 

So how do you detect that those water faces weren't compiled with lit water? 

I obviously can't speak for Mankrip, but I adopted Spike's suggestion of checking the styles too, and confirmed that they were all 0. Lightdata offset for these surfaces was also 0, so whichever light tool was used clearly left the surface uninitialized rather than explicitly setting offset to -1.

My current tests looks like this:

1) Initialize loadmodel->litwater to false at the start of Mod_LoadFaces.

2) Check lightdata offset as standard; if it's -1 there's no lightmap so none of the rest applies.

3) As an additional check test all styles for 0 and lightdata offset for also 0; if this check passes then we have an uninitialized surface so also no lightmap.

4) Otherwise there is a lightmap; at this stage check if the surface has a water texture and if so loadmodel->litwater goes to true.

5) Finally when building the surface polys and lightmaps, I set surf->litwater to the value of m->litwater.

It's clear what happened here; the code that correctly initializes the face for no light (https://github.com/id-Software/Quake-Tools/blob/master/qutils/LIGHT/LTFACE.C#L489) must have been removed from whicever tool was used for lighting this map. 

ericw: Detecting maps compiled with lit liquids.

mh: whichever light tool was used clearly left the surface uninitialized rather than explicitly setting offset to -1

That's not a problem, because there's no need to initialize the surface cache parameters since they should be ignored on non-lit surfaces. See my code on the link above.

My current tests looks like this:

1) Initialize loadmodel->litwater to false at the start of Mod_LoadFaces.

That is bad. Non-lit surfaces are defined on a per-texture basis, and it's perfectly possible for a map to be compiled with both unlit slime and lit water, for example. The engine should not use a global setting for this.

4) Otherwise there is a lightmap; at this stage check if the surface has a water texture and if so loadmodel->litwater goes to true.

There's no need to tie the engine's lightmap usage to texture naming conventions.
One of the potential ideas I have is to implement support for non-subdivided unlit surfaces with regular textures: for example, surfaces using fully black textures (or textures with fullbright texels only) could be rendered much faster this way.

the code that correctly initializes the face for no light (...) must have been removed from whicever tool was used for lighting this map.

It was not removed, because what truly defines the lack of lighting on specific surfaces is not the face data. The map was correctly set to use no lighting. 

TEX_SPECIAL works cleanly. I guess the fact that it's unused in the engine made it easy to miss.

That's not a problem, because there's no need to initialize the surface cache parameters since they should be ignored on non-lit surfaces. See my code on the link above.

I'm actually talking about the face having been left unitialized by the light tool, not by the engine.

If you check the link I posted to LightFace (https://github.com/id-Software/Quake-Tools/blob/master/qutils/LIGHT/LTFACE.C#L489) you'll see that what it does is first set offset to -1 and all styles to 255, then does the TEX_SPECIAL check.

A face with TEX_SPECIAL should also have offset -1 and styles 255. death32c has faces with TEX_SPECIAL and offset 0, styles 0.

The only concern I have about using TEX_SPECIAL is that it's set by the bsp tool, not the lighting tool. This may be purely theoretical: are there any bsp tools that don't set TEX_SPECIAL? 

nice catch, those water faces in death32c have the TEX_SPECIAL flag set on their texinfo. So I agree there should be no need to have special handling for styles = (0 0 0 0) and lightofs = 0; seeing TEX_SPECIAL is enough to indicate that they are meant to be rendered fullbright. 

Lest my questioning be thought of as criticism, I'm actually really happy with the outcome of this. I was able to simplify a bunch of horrible code and the case where one liquid type is lit but another unlit just falls out naturally from it.

It's rare in Quake that the right solution turns out to be so simple, but this is one of those moments. 

Pritchard, awesome beta testing work you did in identifying the situation causing the problem.

I've identified and solved the issue.

@mh @mankrip @ericw - pretty cool discussion sorting out the inner mysteries of lit water. It's nice to see lit water look like it getting closer to go "prime time" from the bsp editor to the compiler tools to the engine. 

I guess the only real outstanding questions I have are (firstly) relating to dynamic lights.

Specifically: should unlit water surfaces receive dynamic lights?

Now, I would LOVE to put dynamic lighting on all water surfaces, lit or not. But it's admittedly easy for me because I've decoupled dynamic lights from lightmaps - it's just commenting out one line of code. (This also made it easy for me to add dynamic lights to BSP brush models.)

The other one is interaction of lightmaps and translucent water.

I've pretty much decided that translucent objects of all kinds (water, glass, etc) don't get lit - they're translucent so light goes through them rather than reflects off them. So instead they're drawn fullbright but with translucency. 

I think this has been brought up before, and I stand by what I said back then. Lighting translucent surfaces is the way to go. I think it would look more accurate, considering that surfaces which are semi-opaque (i.e. 99% of water/brush alpha uses) will still catch/reflect some light in real life.

Also, having to choose between lit water and transparent water would suck. 

This may be purely theoretical: are there any bsp tools that don't set TEX_SPECIAL?

The only one I know of is the -splitspecial flag from TreeQBSP (which is in tyrutils, as well as Bengt's TreeQBSP). TxQBSP (as well as id qbsp) always set TEX_SPECIAL for texture names starting with * or sky in map.c:FindTexinfo.

I guess if someone happened to compile a map with TreeQBSP/tyrutils and used "-splitspecial", and then used whatever light tool was used for death32c.bsp, we could have false positives again, but it seems pretty unlikely. 

The SQLauncher is AWESOME.

I've been renaming all my map titles in the Quake folder to the actual name of the maps, and I can just use SQLauncher to play them. It takes a bit more time renaming everything, but it's much neater. And you will actually the map title.

Quake Injector is a bit buggy, I just prefer to download maps from Quaddicted one at a time so I know what I'm getting. 

The SQLauncher is AWESOME.

I've been renaming all my map titles in the Quake folder to the actual name of the maps, and I can just use SQLauncher to play them. It takes a bit more time renaming everything, but it's much neater. And you will actually the map title.

Quake Injector is a bit buggy, I just prefer to download maps from Quaddicted one at a time so I know what I'm getting. 

mh: should unlit water surfaces receive dynamic lights?

In the software renderer, that's impossible; surface caches needs surface subdivision.

ericw: I guess if someone happened to compile a map with TreeQBSP/tyrutils and used "-splitspecial", and then used whatever light tool was used for death32c.bsp, we could have false positives again

It wouldn't be a false positive. It would be a true positive, because the lack of the TEX_SPECIAL flag would make the light compiler treat liquid surfaces identically to regular surfaces. The only problem would be the lack of backlights, which can be solved by recompiling the lighting alone, without modifying the BSP data. 

Specifically: should unlit water surfaces receive dynamic lights?
I'd say it's up to each engine. I'd personally leave unlit water without dynamic lights.

I've pretty much decided that translucent objects of all kinds (water, glass, etc) don't get lit - they're translucent so light goes through them rather than reflects off them. So instead they're drawn fullbright but with translucency.
I'm with Pritchard, IMHO semi-transparent glass and lit water should still get lightmapped. Fitzquake 0.85 does lightmaps on func_'s with the alpha key < 1, though I'm not sure how many maps make use of lightmapping on glass, Fifth & I exploited it in ad_tfuma.bsp for example: http://i.imgur.com/rhO6awC.jpg

The logic, I guess, is that you can see shadows on dirty, scratched up windows.

It wouldn't be a false positive. It would be a true positive, because the lack of the TEX_SPECIAL flag would make the light compiler treat liquid surfaces identically to regular surfaces.
Ah right.. in that case, even the vanilla id light.exe would generate lightmaps for the water/sky. I guess a better question is, how many released maps (e.g. in quaddicted, or idgames) used TreeQBSP's -splitspecial option, and thus have lightmaps for their liquids? This would be a time when it's handy to have a mirror of quaddicted and run a script across all of the maps.

I'm guessing the answer is "0". 

translucent water *has* to get lightmapped. In fact, the translucency is going to be essential to sell the lightmapping as convincing, otherwise water is going to look like pea soup. 

If you can see the surface, then it reflects light. A certain amount of light passes through it, but not all of it.

Besides, like I said earlier, translucent objects (or any objects, really) glowing in the dark doesn't make any sense in a lot of cases. 

Wondering if this is even possible from an engine perspective? One of my answers in sock's recent survey post:

5. Are you put off by certain monster (horde) setups?

No. I don't love excessive in-fighting though. It's frustrating and I wish modern source ports could have a "slider" for in-fighting. Perhaps: "never>rarely>normal"  

That would get very buggy very fast. Play a map like ad_tfuma, it opens with a bunch of infighting. If that was disabled it wouldn't be the same. 

Infighting doesn't come from the engine anyway, it comes from the QC. It would be totally inappropriate to put any kind of control over it into the engine.