Classic Doom in Doom 3: BFG Edition

Graf Zahl said:

1.11 is the proper number here.

It's still decimally wrong (and thus confusing). 1.1 = 1.10 < 1.11 < 1.2. If Ultimate Doom were 1.09, then yeah, 1.10 would have been good.

printz said:

It's still decimally wrong (and thus confusing). 1.1 = 1.10 < 1.11 < 1.2. If Ultimate Doom were 1.09, then yeah, 1.10 would have been good.

This only works if it was supposed to be a decimal value. It isn't, as it is not supposed to be read mathematically (the dot is a separator, not a decimal point).

I hereby propose 1.110. It appears "larger" than both 1.91 and 1.92, if NOT interpreted as a decimal suffix.

Quasar said:

Not true.

When a thing is in deferred removal state, its thinker function is set to P_RemoveThinkerDelayed. This compares as not equal to P_MobjThinker, so, everywhere looking for an mobj, or any other type of thinker, skips a thinker in this state:

for(th = thinkercap.next; th != &thinkercap; th = th->next) {
   if(th->function == P_MobjThinker) {
      mobj_t *mo = (mobj_t *)th;
      ...
   }
}

It *does* continue to add some overhead to the list iteration, but other than that, no time is spent processing such objects.

The point is that the developer must test for this every time the thinker list is iterated, for example A_KeenDie(). In my book this is no different from testing whether it has already been removed by comparing the function ptr with (think_t) (-1). Consequently it achieves nothing.

DaniJ said:

The point is that the developer must test for this every time the thinker list is iterated.

This must be done anyway because the thinker list also contains non-mobjs.

Exactly. Hence my point. The fact that Boom goes to the extent of creating separate lists for different types of thinker further shows how poorly designed this whole subsystem is: if there is a separate list for thinkers of type X then why do I test the type of each N when iterating said list?

(the reason being to determine whether the thinker has been "flagged" for removal, yet in the process this invalidates the lists and thus half the benefit in separating the thinkers in the first place)

That's not poor design on Boom's part. That's a necessity for maintaining demo compatibility.

I think only MBF implemented special lists, one for hostile monsters, one for friendly monsters. And both are not used for thinker iteration but only for finding targets more quickly if I'm not mistaken.

Everything, including the stuff in those above lists is mashed together in one large thinker list and it needs to remain like that if demo compatibility is important.


There's no denying that the original implementation was pretty bad - but does it really matter in the end? Even with a few 1000 objects in a list the time spent doing these checks is completely inconsequential.

Graf Zahl said:

That's not poor design on Boom's part. That's a necessity for maintaining demo compatibility.

I admit I'm ignorant of the needs of demo compatibility and how this applies to the thinker list. My point was only ever that this subsystem's design was left pretty wanting and that it was an early architectural decision they never got around to addressing.

There's no denying that the original implementation was pretty bad - but does it really matter in the end? Even with a few 1000 objects in a list the time spent doing these checks is completely inconsequential.

As I said earlier in the thread, this isn't about the minor performance impact of this. The issue I have is the load this places on the developer having to be mindful of the esoteric rules of the subsystem.

From Doomsday's point of view, with us imminently beginning work on binding of the playsim to the scripting core, these unnecessary cognitive overheads need to be dealt with so that we don't pass them on to mod authors.

Edward850 said:

This only works if it was supposed to be a decimal value. It isn't, as it is not supposed to be read mathematically (the dot is a separator, not a decimal point).

Of course, this is all complicated by Id Software going 1.6, 1.666, 1.7, 1.7a...

Anyway, it doesn't matter.

Does this compile with minimal/easy to obtain dependencies, on Windows? In other words, do I get the BFG Edition classic Doom engine for free with this?

Given that some dude fixed a bug 10 hours after code release, I guess so?

Gez said:

Given that some dude fixed a bug 10 hours after code release, I guess so?

To be fair, that "bug" must have been introduced after the forced Cpp-fication of the code. The original Doom had:

char *sprnames[NUMSPRITES] = {
    "TROO","SHTG","PUNG","PISG","PISF","SHTF","SHT2","CHGG","CHGF","MISG",
    "MISF","SAWG","PLSG","PLSF","BFGG","BFGF","BLUD","PUFF","BAL1","BAL2",
    "PLSS","PLSE","MISL","BFS1","BFE1","BFE2","TFOG","IFOG","PLAY","POSS",
    "SPOS","VILE","FIRE","FATB","FBXP","SKEL","MANF","FATT","CPOS","SARG",
    "HEAD","BAL7","BOSS","BOS2","SKUL","SPID","BSPI","APLS","APBX","CYBR",
    "PAIN","SSWV","KEEN","BBRN","BOSF","ARM1","ARM2","BAR1","BEXP","FCAN",
    "BON1","BON2","BKEY","RKEY","YKEY","BSKU","RSKU","YSKU","STIM","MEDI",
    "SOUL","PINV","PSTR","PINS","MEGA","SUIT","PMAP","PVIS","CLIP","AMMO",
    "ROCK","BROK","CELL","CELP","SHEL","SBOX","BPAK","BFUG","MGUN","CSAW",
    "LAUN","PLAS","SHOT","SGN2","COLU","SMT2","GOR1","POL2","POL5","POL4",
    "POL3","POL1","POL6","GOR2","GOR3","GOR4","GOR5","SMIT","COL1","COL2",
    "COL3","COL4","CAND","CBRA","COL6","TRE1","TRE2","ELEC","CEYE","FSKU",
    "COL5","TBLU","TGRN","TRED","SMBT","SMGT","SMRT","HDB1","HDB2","HDB3",
    "HDB4","HDB5","HDB6","POB1","POB2","BRS1","TLMP","TLP2"
};

and not

const char * const sprnames[NUMSPRITES] = { "TROO",..... }

Apparently, the former always works, while the latter doesn't.

Maes said:

To be fair, that "bug" must have been introduced after the forced Cpp-fication of the code. The original Doom had:

Apparently, the former always works, while the latter doesn't.

It's nothing to do with the change to const *const; that was done to avoid modern compilers' warnings about assigning string constants to non-constant pointers (old compilers allowed this silently and it often resulted in undefined behavior - string constants may be read-only depending on the implementation). It's also nothing to do with C++; this same issue was detected and fixed in BOOM, because it crashed under DJGPP.

It's everything to do with the fact that R_InitSprites iterates over sprnames looking for a NULL terminator at the end of the array. As you can plainly see, there is no such.

Why did it work in vanilla? Guess what was next in memory after the sprnames array... states. What's the first state look like?

{ SPR_TROO, ... }, // S_NULL

And what is the value of SPR_TROO? Why, it's zero good man! So the code worked by accident, by relying on the relative position of sprnames and states in memory. Under Watcom's linkage, they were always consecutive in the executable image, with no padding between them.

Mega heh.

Quasar said:

And what is the value of SPR_TROO? Why, it's zero good man! So the code worked by accident, by relying on the relative position of sprnames and states in memory. Under Watcom's linkage, they were always consecutive in the executable image, with no padding between them.

Same as how Eternity's Cardboard renderer used to work until tweaked recently.

printz said:

Same as how Eternity's Cardboard renderer used to work until tweaked recently.

What are you talking about?

Quasar said:

What are you talking about?

The way columns were read in cardboard before ryan caught the problem in OpenBSD?

printz said:

The way columns were read in cardboard before ryan caught the problem in OpenBSD?

Well, uhh, ok, but:

• That problem is not recent. It had been in the code for a year or two already.
  
• That problem was nowhere as simple as this one.
  
• I had nothing to do with that problem's existence.
  
• It's not particularly comparable since it would always commit an out-of-bounds access; whether or not it mattered in this case didn't depend on the alignment of stuff in the executable file, but upon the size of blocks on the heap and continuity (or lack thereof) between allocations in the process address space.
  

Has the Doom 1.11 executable binary been released anywhere free of charge? Or maybe an upgrade patch from 1.9.

You have to pay for Doom 3: BFG Edition, simple as that.

chungy said:

You have to pay for Doom 3: BFG Edition, simple as that.

So there's no upgrade patch yet? I know I don't have to pay if I can download the source code and compile it, but I was wishing for a standard readily compiled version (with a fixed timestamp).

Doom v1.11 only works from within Doom 3. It doesn't communicate with the OS to do all the sound and video output; it communicates with Doom 3, which serves as its shell.

There's not even a separate executable for it: its entry point is normally reachable only throught the BFG launcher, and even though they didn't go out of their way to remove the support for vanilla command-line parameters from it, I don't know how (and if) they can be used. So functionality like demo recording, loading PWADs etc. is only theoretical It could be broken as hell, for all we know.

So far, I think that only Sodaholic managed to load PWADs with it, and that's only through the slow and painful method of IWAD merging...having an independent executable would help at least with testing that stuff, but instead of waiting for someone to make one out of good will, perhaps we at DW should take action and make a reference one that actually compiles without going into dependency hunting? Or update our source ports to match its changes, e.g. recognize the nerve mission pack, support its new demo format etc.?

The thing I'm curious about is the Master Levels support. It looks like it's meant to support a single wad with 21 levels, from MAP01 to MAP21. This wad doesn't seem to officially exist in PC land; only on the PlayStation Network.

struct ExpansionData {

	enum { IWAD = 0, PWAD = 1 }	type;
	GameMode_t					gameMode;
	GameMission_t				pack_type;
	const char *				expansionName;
	const char *				iWadFilename;
	const char *				pWadFilename;
	const char *				saveImageFile;
	const char **				mapNames;


};

	const ExpansionData App_Expansion_Data_Local[] = {
		{	ExpansionData::IWAD, retail,		doom,			
			"DOOM",								DOOMWADDIR"DOOM.WAD",		
			NULL,							"base/textures/DOOMICON.PNG", 
			Doom_MapNames },
		{	ExpansionData::IWAD, commercial,	doom2,			
			"DOOM 2",							DOOMWADDIR"DOOM2.WAD",		
			NULL,							"base/textures/DOOM2ICON.PNG", 
			Doom2_MapNames },
		{	ExpansionData::IWAD, commercial,	pack_tnt,		
			"FINAL DOOM: TNT EVILUTION",		DOOMWADDIR"TNT.WAD",		
			NULL,							"base/textures/TNTICON.PNG", 
			TNT_MapNames },
		{	ExpansionData::IWAD, commercial,	pack_plut,		
			"FINAL DOOM: PLUTONIA EXPERIMENT",	DOOMWADDIR"PLUTONIA.WAD",	
			NULL,							"base/textures/PLUTICON.PNG", 
			Plut_MapNames },
		{	ExpansionData::PWAD, commercial,	pack_master,	
			"DOOM 2: MASTER LEVELS",			DOOMWADDIR"DOOM2.WAD",		
			DOOMWADDIR"MASTERLEVELS.WAD",	"base/textures/MASTICON.PNG", 
			Mast_MapNames },
		{	ExpansionData::PWAD, commercial,	pack_nerve,		
			"DOOM 2: NO REST FOR THE LIVING",	DOOMWADDIR"DOOM2.WAD",		
			DOOMWADDIR"NERVE.WAD",			"base/textures/NERVEICON.PNG", 
			Nerve_MapNames },
	};

Gez said:

Doom v1.11 only works from within Doom 3. It doesn't communicate with the OS to do all the sound and video output; it communicates with Doom 3, which serves as its shell.

I wonder if sound and music code can still be extracted from Doom 3-BFG and Doom 1.11 and used in any GPLv3 Doom source ports, instead of SDL Mixer or FMod.

That still wouldn't help anyone using GPL v2, since the code is GPL v3.

Graf Zahl said:

That still wouldn't help anyone using GPL v2, since the code is GPL v3.

Yeah but it can be upgraded; I think GhostlyDeath already did it.

printz said:

I wonder if sound and music code can still be extracted from Doom 3-BFG and Doom 1.11 and used in any GPLv3 Doom source ports, instead of SDL Mixer or FMod.

I saw a lot of commented out-stuff to API-specific headers .... so probably it's more of the same. I didn't notice if they use the vanilla mixer though...

printz said:

Yeah but it can be upgraded; I think GhostlyDeath already did it.

Sure, but some people do not want to because GPL v3 comes with even more baggage than v2.

Actually the code is not GPLv3 at all.

Due to the modifications that Zenimax insists on making to the GPLv3 on all of their recent code releases, the code is actually under its own incompatible license, which is a variant of GPLv3.

That's great for operating on the code itself, but it means you cannot combine the code with anything under the standard GPLv3. I doubt id is even aware of the problem that creates, or that they care.

ADDITIONAL TERMS:  The Doom 3 GPL Source Code is also subject to
certain additional terms. You should have received a copy of these
additional terms immediately following the terms and conditions of the 
GNU GPL which accompanied the Doom 3 Source Code.  If not, please
request a copy in writing from id Software at id Software LLC, c/o 
ZeniMax Media Inc., Suite 120, Rockville, Maryland 20850 USA.