Talk:Invulnerability colormap bug

From DoomWiki.org

A screenshot. That's a very good idea — it makes the article more self-contained. Ryan W 21:04, 23 Jun 2005 (UTC)

COLORMAP/PLAYPAL[edit]

COLORMAP controls the invulnerability powerup's colouring effect. COLORAMP is only applied to the player's view window. But it does not affect the sky presumably because it is also used for light diminishing. And having the sky be changed by sector lighting would look weird.

PLAYPAL would not be desirable because this affects everything. Viewing the automap, status bar and menus would be horrible. And it'd also be subject to being removed by gamma change (F11)

g6672D 16:24, 20 January 2007 (UTC)

But that's what the article says ... ? Fraggle 02:38, 21 January 2007 (UTC)

No details![edit]

But it doesn't tell you why it's known to be a bug. It also doesn't state why it occurs.

Hmm. Lets look at linuxdoom 1.10 (GPL). R_DrawPlanes() in r_plane.c:401:
// Sky is allways drawn full bright,
//  i.e. colormaps[0] is used.
// Because of this hack, sky is not affected
//  by INVUL inverse mapping.
dc_colormap = colormaps;
Here, dc_colormap is a variable that will be used by R_DrawColumn (which will draw a single vertical slice of any wall, sprite, or sky). On the other hand, regular flats when handled in R_DrawPlanes eventually pass control to R_MapPlane() in r_plane.c:161:
    if (fixedcolormap)
	ds_colormap = fixedcolormap;
    else
    {
	index = distance >> LIGHTZSHIFT;
	
	if (index >= MAXLIGHTZ )
	    index = MAXLIGHTZ-1;

	ds_colormap = planezlight[index];
    }
fixedcolormap is set (see p_user.c:363, where it's first set in the player struct) to non-zero when invuln or lite amp is active, giving precedence to invuln. So there are two override rules for colormap at work here; one originating in p_user.c:363 sets the inverse colormap for invuln, or, if no invuln, the full brite colormap for lite amp. This is applied to just about everything visible. The second, in r_plane.c:401, sets the fullbrite colormap always, applied only to sky textures. However, the second doesn't even look to see if the first is active; the sky codeblock doesn't examine fixedcolormap at all before setting fullbrite, unlike all other wall, sprite, and flat rendering. Unless I've missed something (which is highly likely; the source is a mess and I'm not very familiar with it), there doesn't appear to be any particular indication that this was a mistake other than the one comment given. I'm inclined to think it's not a bug, as it is very obvious and certainly would have shown up at some point during testing, and fixing it is very simple; the sky code in R_DrawPlanes() simply needs to check fixedcolormap before setting fullbrite. If it was a bug I think it certainly would have been fixed by 1.9. 71.58.109.233 22:47, 14 September 2007 (UTC)
While I personally am forced to approach bugs from a non-programmer's perspective, I see several possible objections to your reasoning:
  • That comment (indeed many of the comments) could well have been added during the code release, which means the exact location of the bug may or may not have already been known.  There is at least one other example of a copy-paste typo surviving until after v1.9.
  • Doom change log and Masters of Doom (the book itself, not our article) suggest that, after the registered version was finally finished, the list of priorities was "fix fatal bugs" followed by "fix bugs materially affecting gameplay" followed by "fix the — oh shit, time to work on Quake instead.  Talk to the hand."  So no, it wouldn't necessarily have been addressed by v1.9.
  • If you look in the original 3 episodes, there are surprisingly few places where the player typically views large chunks of the sky while invulnerable.  Maybe they really didn't notice!  (I realize this doesn't excuse ignoring it after the release, when 5 million new playtesters were suddenly acquired.)
  • People who know C insist that checking for array overflow is a completely straightforward procedure, yet in case after case it turns out not to be done in the Doom code.  Should we therefore say that all the overflow-related problems aren't bugs, because they must have been obvious at the time?
Mind you, it's always good when this kind of analysis is added to one of our technical articles, even on the talk page.   :>      Ryan W 04:13, 15 September 2007 (UTC)
Lots of good thoughts; there are certainly a number of possible answers here. I of course can't be conclusive, but one thing I think, is that there's not enough information to support "this was found to be a bug after the release of the source code." as in the article text. Maybe a weaker statement would be appropriate? Either that or whoever found the better evidence that I missed could explain it. 71.58.109.233 06:06, 15 September 2007 (UTC)
In certain cases, a few source port authors have been staring at these articles since 2005 without "being conclusive", so I suspect that the rest of us needn't feel ashamed quite yet.  :>   About the statement you quote, I entirely agree.  (In general, a remark of the form "the community changed its opinion at such-and-such a time" is IMHO tricky to work with.  It is quite difficult to get a consensus judgement on the present, let alone when the leading lights of the group have had an extra 2-8 years to drift away from Doom for whatever reason.)    Ryan W 07:32, 15 September 2007 (UTC)
I certianly think this is a bug. Why would the programmers purposefully leave the sky like that? Then again, the fix is extremely obvious, so it couldn't have been that it was too hard to fix. All you would have to do is change this:

dc_colormap = colormaps;

to this:

dc_colormap = ((fixedcolormap)?(fixedcolormap):(colormaps));

-Wagi 69.51.157.227 15:27, 17 January 2008 (UTC)