player corpses show correct colors bugfix

[TLWmarko]

GLQuake introduced a bug with player corpses always showing the default ranger colors, so I've restored this feature that used to work in WinQuake (and probably DOSQuake).

This patch fixes the corpse colors specifically, but the way WinQuake works is more flexible which would allow any model to use the colors a certain player has selected, but that's a more significant change which requires creating and freeing new textures.

[sezero]

This removes commit 57a32b567fd822a6ba2a2bb0a1d19b005ea44593 but I guess that won't be needed after this.

@andrei-drexler, @temx, @ericwa: Can you please review?

[Shpoike]

won't fix QuakeRally's car colours, won't fix team-coloured sentryguns in TF, etc. will glitch horribly when the player becomes a gibbed head/etc.

For qss I recently fixed it in https://github.com/Shpoike/Quakespasm/commit/c52a165ca55b825cedb376092d391b0f42e98a48 which should basically behave exactly like winquake would in this regard, while also supporting replacement textures etc... took a few other commits to fix my bugs, but oh well... anyway, basic idea is to create a lookup+cache for texture+colours->remappedtexture instead of the silly playertextures array - quakespasm's texture manager was made to do reloading etc so this isn't too hard to add. replacement textures are more akward and need base+upper+lower textures with them blended+added, which kinda requires glsl when alpha is in use. the upper+lower are then tinted. you could use this for 8bit textures too, but you'd need to handle quake's non-linear colour ranges (particularly range 6) if you want it to match winquake properly - my code instead just uses a simple multiplier there for custom RGB player colours, resulting in different methods for different types of textures.

[TLWmarko]

I said as much in my description @Shpoike

I'd much rather see this small fix come in now rather than delay/risk it not happening because the more proper fix might be considered too substantial or something, and there's no conflict in doing the more proper one later. This one was quick and easy for now, and allows a feature in an upcoming map pack.

[ericwa]

Agree with the idea of the easy fix and then importing Spike's more complete fix later. I'm not familiar with the shirt/pants stuff at all though.

I think I was able to repro the issue with stock QS, is this the bug we're talking about?

• start two copies of QS, create a multiplayer game in one and join it in the other
• set custom shirt/pants colors for both players
• when one player dies, and clicks to respawn, the corpse reverts to the default ranger colors

[TLWmarko]

Yeah that's the bug! I'm totally down for doing the more complete fix later as well

[sezero]

@ericwa: Do you approve this patch, then?

[ericwa]

Seems like it's not working for me, I'm getting the same behaviour as before.

The player corpse previously had the correct colors, but I just clicked to respawn in that player's QS window (not pictured).

I'm debugging the QS window which is looking at the corpse, I think it's the R_DrawAliasModel invocation that's drawing the corpse. cl_entities[e->vcolormap].model is NULL.

[TLWmarko]

Well that was unexpected. I'll take another look at it later today, maybe I somehow missed something when cleaning up for the commit

[Shpoike]

For the uninitiated (yay! a wall of text to ignore!): The old software renderer did lighting by having a 25664 lookup proving sourcepaletteindexlightlevel->outputpaletteindex (the 'vid.colormap' pointer points to this table) Player colours were provided for by creating a per-player colourmap where palette-rows 1 and 6 are swapped out for the rows indicated by your top+bottom colours, the renderer can them simply switch to using the per-player colourmap instead and the player comes out a different colour simply by swapping out a pointer or two making the whole thing really cheap to do. The networked colormap entity field is thus simply which player's colourmap to use - effectively the player slot index, with 0 meaning the default colourmap. Side note: This also means that corpses will also change colours when the player they were spawned from later changes their colour (which may reveal spies in TF a little too easily). There is a dp extension for ent.colormap=4096|(upper*16)|lower; which gives explicit top/bottom colours regardless of player slots, but finding out the player's top colour is a separate issue, as is networking the fact that its no longer a player index.

this is a problem for glquake (basically was stuck to gl1.1) which was basically forced to use 32bit textures and thus had no such ability to change the colourmap(or palette) so freely. [Side note: in theory GL_EXT_paletted_texture would be able to provide the ability to swap out the texture's palette to recolour the respective indexes, but there's a reason every single quake engine has stripped all attempts to use it...] Instead, glquake used a playertexture lookup which ONLY works for the specific player entity, and ignores all the issues that result from corpses or quakerally cars etc using different models/skins with the same colourmap on a plethora of other entities.

For QS, the 'simple' fix is to just strip the playertextures array and all references to it and simply call TexMgr_ReloadImage on the mdls texture just before rendering it. In practise this is likely to incur an unacceptable slowdown, so QSS has a lookup of texture+colours->newtexture and basically just makes a private copy of the texture that it can then ReloadImage the once to colour it properly and then reuse until something else reuses it for something else. Doing it on an on-demand basis like that instead of unconditionally for every player will probably actually result in fewer such textures in team games. However there will always be the odd player that has some alias to constantly change their colours that might consume many more than would otherwise be used (though trivial ones will not care when that annoying person keeps switching back/forth between two colours).

For QSS, much of the complexity was to support 24bit skins (with the tinted parts ofs the diffusemap blackish with two additional textures to say which pixels need how much of the player's relevant colour added in. It should be noted that this layers approach will not deal very well with non-linear ranges - read: row 6 that contains both yellows and reds, hence why QSS still needs to retain the whole ReloadImage stuff. It should also be noted that QS does not actually support 24bit model skins anyway so this is unlikely to be a concern, simplifying the implementation and avoiding any/all issues when glsl is not available, thus adding QSS's colourmap stuff to QS too basically becomes a case of stripping all of QS's playertexture stuff from cl_parse.c, tweaking the small bit in r_alias.c, and porting over the relevant texmgr changes from qss to provide the recolouring cache, ignoring any and all references to md3/md5/iqm formats or upper/lower textures etc, any mdl vertex array stuff is also irrelevant to this change - it is simply a case of swapping out a texture for a tinted one. The only real complication remaining is tidying stuff up cleanly without any dead pointers (and avoiding any 'color 0xRRGGBB' minefields in ported QSS code).

For reproduction, easiest way to test it is to load up quakerally and play qrdemo1.dem or whatever its called. There should be 4 players about to have a race changing their models/skins/colours a bit (teleport particles), yet they basically all stay grey in QS. The car entities seen are not the actual player entities, so are logically no different from eg corpses in id1 while also guaranteeing that the playertextures array never even saw that specific model+colour combo (eg connecting while someone is a gibbed head).

[TLWmarko]

Thanks @Shpoike for the additional details! I'll start looking at porting the non-24bit support stuff.

I was able to reproduce the bug after I had a theory of what might be wrong. It happens when the player whose corpse is on screen is outside the pvs of the rendering player. My theory was that Quake doesn't send info about the entities which aren't in the pvs, which might have fields like .model cleared, but it doesn't entirely explain why even the server has the same bug. Either case it's still true that when the dead player is not in pvs the corpse will have default colors, but when they walk back into pvs the correct colors become visible again.

If someone has a better idea for checking that the models of the corpse and player matching, this could be remedied and this simple version of the corpse colors could be implemented.

[TLWmarko]

So my suspicion turned out to be right, and I'm currently not seeing a solution for the bug in this "simple version" of the feature.

I was able to mostly fix it by adding a similar way of comparing models as the vcolormap field in this PR, there will still be a case of when a new player joins, they might not have seen the player at any point whose corpse is lying on the floor, meaning the server will never have sent the info about that players modelindex, so up until the players meet atleast once the corpse will have default colors.

Unless someone has an idea for solving that I'm feeling like this PR should be closed and I'll instead just look at the complete solution.

[sezero]

OK, closing.