cl_visedicts: are duplicate entries OK/expected?
-
- Posts: 22
- Joined: Mon Apr 28, 2014 8:34 pm
- Location: San Carlos, CA
- Contact:
cl_visedicts: are duplicate entries OK/expected?
As a learning experience (and out of curiosity) I've been looking into some of the known issues of the reQuiem engine. Some I can track down just from general C debugging, but in other cases I need to pick up more understanding of the Quake data structures and how they're used.
In one case, I see a linked list that is getting corrupted because it is processing cl_visedicts without taking into account that there could be multiple elements of that array that point to the same object. What I don't yet know is whether:
a) it is bad/unexpected for cl_visedicts to have duplicate entries
or
b) duplicates are OK and the linked-list manipulation code should take that possibility into account.
Can someone give me a push in the right direction?
(If you want to climb further back along my debugging logic, it's this issue: https://github.com/SpiritQuaddicted/reQuiem/issues/9 )
In one case, I see a linked list that is getting corrupted because it is processing cl_visedicts without taking into account that there could be multiple elements of that array that point to the same object. What I don't yet know is whether:
a) it is bad/unexpected for cl_visedicts to have duplicate entries
or
b) duplicates are OK and the linked-list manipulation code should take that possibility into account.
Can someone give me a push in the right direction?
(If you want to climb further back along my debugging logic, it's this issue: https://github.com/SpiritQuaddicted/reQuiem/issues/9 )
Re: cl_visedicts: are duplicate entries OK/expected?
however you look at it, dupes are pointless and inefficient. the only way that it might actually do anything is if .alpha isn't 1, but there's better ways to handle that.
if you have multiple scenes being drawn (ie: recursing through water leafs) then you should probably remove any entities already on the list just so you don't draw them twice or whatever.
I can't think of any specific places where dupes would actually be crashy, but then its been a long time since I seriously looked at NQ code. QW generates the data in-place, instead of using pointers to some other random bits of memory, its cleaner that way.
if you have multiple scenes being drawn (ie: recursing through water leafs) then you should probably remove any entities already on the list just so you don't draw them twice or whatever.
I can't think of any specific places where dupes would actually be crashy, but then its been a long time since I seriously looked at NQ code. QW generates the data in-place, instead of using pointers to some other random bits of memory, its cleaner that way.
-
- Posts: 22
- Joined: Mon Apr 28, 2014 8:34 pm
- Location: San Carlos, CA
- Contact:
Re: cl_visedicts: are duplicate entries OK/expected?
The crashing part is the linked list (skychain), which is something new in reQuiem.
reQuiem's handling of cl_visedicts (which allows/causes duplicate entries) doesn't seem to be notably different from other NQ engines, although of course I could be missing something.
reQuiem's handling of cl_visedicts (which allows/causes duplicate entries) doesn't seem to be notably different from other NQ engines, although of course I could be missing something.
Re: cl_visedicts: are duplicate entries OK/expected?
A thought, right or wrong.
I've looked at the Requiem change log and it appears to have previous builds dated back to 2008. I think prior to the release (early this year?), this served as Spirit's private engine.
*IF* there are previous versions of the source code available, do you have those? You might consider building one a couple of versions ago and see if has the problem and isolate the one that introduced it.
Then focus on the changes/implementation that occurred between those version.
I've looked at the Requiem change log and it appears to have previous builds dated back to 2008. I think prior to the release (early this year?), this served as Spirit's private engine.
*IF* there are previous versions of the source code available, do you have those? You might consider building one a couple of versions ago and see if has the problem and isolate the one that introduced it.
Then focus on the changes/implementation that occurred between those version.
Short version: There shouldn't be duplicate entries on the cl_visedicts, but this doesn't seem likely to cause a crash unless it is a sign of another problem. Then again, Requiem used JoeQuake as the source base as far as I can tell and its particle rendering stuff is touchy.vis_edicts, how I'd describe them wrote: The vis_edicts list are the supposedly visible entities from the server that are not static (like torches or func_illusionary). If I recall correctly, the player himself isn't on this list and if chase_active 1 is enabled, must be manually added.
As best as I know --- 95% certain --- there should never be one entity in vis_edicts list that is listed twice.
The server sends the visible edicts, a complete list of them, as it checks each one in sequence and determines if the player can see them. Because this is a simple "entity #1", "entity #2" for loop duplicates should not be possible.
Having duplicates in the list would render them twice --- which wouldn't cause a problem just slow things down rendering-wise. Duplicates shouldn't cause a crash, but would be in the rendering list twice.
The night is young. How else can I annoy the world before sunsrise? Inquisitive minds want to know ! And if they don't -- well like that ever has stopped me before ..
Re: cl_visedicts: are duplicate entries OK/expected?
Ran Requiem (I compiled it and ran debug version). game czg07 + map czg07 + kill = crash.
So I started up FitzQuake085.exe as a dedicated server as such:
c:\quake\fitzquake085.exe -game czg07 +map czg07 -dedicated 1 -window
In FitzQuake085.exe, I typed "status" which showed me my ip address of "192.168.1.4"
I started Requiem and change the gamedir to czg07. I typed "connect 192.168.1.4" to connect to FitzQuake running in a different window.
I type "kill" and Requiem crashed. But the music kept playing. I set a breakpoint in Visual Studio in the beginning of _Host_Frame. Nothing happened [it didn't hit the breakpoint] (which means the _Host_Frame which runs 200 frames per second +/- was no longer happening). So despite still having music playing and a non-responsive client and Visual Studio did not catch an error or hit a breakpoint. FMOD plays the music and was still running.
Now ...
I did this.
c:\quake\fitzquake085.exe -game czg07 +coop 1 +map czg07 -dedicated 1 -window
When I changed the gamedir in Requiem and connected, I could kill myself as many times as I wanted!!!!! No crashes or anything.
Why would coop 1 make a difference? Maybe Requiem does something special when you die? Does it auto-record demos? Do they save on death? Does it zip them up?
Or maybe something in Requiem only executes when a single player game is running locally?
Do you have a particular reason you suspect visedicts is the problem?
So I started up FitzQuake085.exe as a dedicated server as such:
c:\quake\fitzquake085.exe -game czg07 +map czg07 -dedicated 1 -window
In FitzQuake085.exe, I typed "status" which showed me my ip address of "192.168.1.4"
I started Requiem and change the gamedir to czg07. I typed "connect 192.168.1.4" to connect to FitzQuake running in a different window.
I type "kill" and Requiem crashed. But the music kept playing. I set a breakpoint in Visual Studio in the beginning of _Host_Frame. Nothing happened [it didn't hit the breakpoint] (which means the _Host_Frame which runs 200 frames per second +/- was no longer happening). So despite still having music playing and a non-responsive client and Visual Studio did not catch an error or hit a breakpoint. FMOD plays the music and was still running.
Now ...
I did this.
c:\quake\fitzquake085.exe -game czg07 +coop 1 +map czg07 -dedicated 1 -window
When I changed the gamedir in Requiem and connected, I could kill myself as many times as I wanted!!!!! No crashes or anything.
Why would coop 1 make a difference? Maybe Requiem does something special when you die? Does it auto-record demos? Do they save on death? Does it zip them up?
Or maybe something in Requiem only executes when a single player game is running locally?
Do you have a particular reason you suspect visedicts is the problem?
The night is young. How else can I annoy the world before sunsrise? Inquisitive minds want to know ! And if they don't -- well like that ever has stopped me before ..
Re: cl_visedicts: are duplicate entries OK/expected?
Sadly I only got that one version of the source.Baker wrote:*IF* there are previous versions of the source code available, do you have those? You might consider building one a couple of versions ago and see if has the problem and isolate the one that introduced it.
Improve Quaddicted, send me a pull request: https://github.com/SpiritQuaddicted/Quaddicted-reviews
Re: cl_visedicts: are duplicate entries OK/expected?
or in other words, your server code shuts down everything, purges the world model, displays the loading screen, and draws the world when it has already been destroyed...reQuiemd.exe!R_DrawSkyChainOld() Line 224 + 0xb bytes C
reQuiemd.exe!R_DrawSky() Line 1312 C
reQuiemd.exe!R_RenderScene() Line 1599 C
reQuiemd.exe!R_RenderView() Line 1712 C
reQuiemd.exe!V_RenderView(qboolean force_refdef_recalc=false) Line 1400 C
reQuiemd.exe!SCR_UpdateScreen() Line 2168 + 0x9 bytes C
reQuiemd.exe!SCR_BeginLoadingPlaque(const char * caption=0x01678b4e) Line 1751 C
reQuiemd.exe!Host_Reconnect_f(cmd_source_t src=SRC_COMMAND) Line 658 + 0x9 bytes C
reQuiemd.exe!Cmd_ExecuteString(const char * text=0x0018f5f8, cmd_source_t src=SRC_COMMAND) Line 1023 + 0xc bytes C
reQuiemd.exe!SV_SendReconnect(const char * newmap=0x0018f838) Line 2419 + 0xe bytes C
reQuiemd.exe!SV_SpawnServer(const char * server=0x0018f838, const char * startspot=0x00000000) Line 2513 + 0x9 bytes C
reQuiemd.exe!Host_Restart_f(cmd_source_t src=SRC_COMMAND) Line 624 + 0xb bytes C
make sure cl.worldmodel->needload isn't set, and if it is, don't draw it?
using coop instead works because it isn't executing the restart console command when you die, and thus isn't executing console commands at the standard (safe) time.
-
- Posts: 22
- Joined: Mon Apr 28, 2014 8:34 pm
- Location: San Carlos, CA
- Contact:
Re: cl_visedicts: are duplicate entries OK/expected?
Details at https://github.com/SpiritQuaddicted/reQuiem/issues/9Baker wrote: Do you have a particular reason you suspect visedicts is the problem?
Duplicate cl_visedicts entries can cause a loop in skychain. (This part isn't speculation, it's something I observed.)
Last edited by Johnny Law on Wed Jul 09, 2014 3:12 pm, edited 2 times in total.
-
- Posts: 22
- Joined: Mon Apr 28, 2014 8:34 pm
- Location: San Carlos, CA
- Contact:
Re: cl_visedicts: are duplicate entries OK/expected?
Hmm yep, that sounds like it could keep this particular case from happening, but it seems like there's a root cause that should be addressed. There are lots of spots that will call SCR_UpdateScreen, which will (down the callstack) increment r_framecount and add things into cl_visedicts. If multiple of those calls happen between invocations of CL_RelinkEntities, as is happening here, then it's possible to get dups in cl_visedicts.Spike wrote:make sure cl.worldmodel->needload isn't set, and if it is, don't draw it?
Re: cl_visedicts: are duplicate entries OK/expected?
just came across this comment from mh which sounds related:
http://forums.inside3d.com/viewtopic.ph ... 507#p48507
http://forums.inside3d.com/viewtopic.ph ... 507#p48507
-
- Posts: 22
- Joined: Mon Apr 28, 2014 8:34 pm
- Location: San Carlos, CA
- Contact:
Re: cl_visedicts: are duplicate entries OK/expected?
Welp! That's confirmation, thanks.
Guess the most prudent fix for reQuiem would be to change the skychain management so that it doesn't implode when there are duplicate cl_visedicts elements.
Guess the most prudent fix for reQuiem would be to change the skychain management so that it doesn't implode when there are duplicate cl_visedicts elements.
Re: cl_visedicts: are duplicate entries OK/expected?
I was able to stop the crash in Requiem ...
There is a great function in Requiem:
1. Find all locations of cl_numvisedicts++
2. The code will look like this:
OR
OR
And replace it with the equivalent of this:
Will stop the crash.
There is a great function in Requiem:
Code: Select all
void CL_LinkEntity(entity_t *ent)
{
extern qboolean scr_drawloading; // Baker added
if (scr_drawloading) // Baker added
return; // Baker added
if (cl_numvisedicts < MAX_VISEDICTS)
{
cl_visedicts[cl_numvisedicts++] = ent;
}
}
2. The code will look like this:
Code: Select all
cl_visedicts[cl_numvisedicts] = ent;
cl_numvisedicts++;
Code: Select all
cl_visedicts[cl_numvisedicts++] = ent;
Code: Select all
cl_visedicts[cl_numvisedicts++] = pent;
Code: Select all
#if 1
CL_LinkEntity (ent); // Runs this function, which will check scr_drawloading.
#else
if (cl_numvisedicts < MAX_VISEDICTS)
{
cl_visedicts[cl_numvisedicts] = ent;
cl_numvisedicts++;
}
#endif
The night is young. How else can I annoy the world before sunsrise? Inquisitive minds want to know ! And if they don't -- well like that ever has stopped me before ..
Re: cl_visedicts: are duplicate entries OK/expected?
Here is why I believe Requiem is vulnerable to this problem:
1) FitzQuake runs particles once per frame.
2) JoeQuake runs particles when rendering.
FitzQuake-ish engine:
JoeQuake (Requiem inherited ...) does it during the rendering process.
SCR_Update --- > R_RenderView --> R_DrawParticles --> Classic_DrawParticles|QMB_DrawParticles (those 2 functions spawns particles)
This function runs particle physics:
1) FitzQuake runs particles once per frame.
2) JoeQuake runs particles when rendering.
FitzQuake-ish engine:
Code: Select all
void _Host_Frame (float time)
{
...
// update video
if (host_speeds.value)
time1 = Sys_DoubleTime ();
{
SCR_UpdateScreen ();
if (!sv.frozen)
CL_RunParticles (); //johnfitz -- seperated from rendering
}
...
SCR_Update --- > R_RenderView --> R_DrawParticles --> Classic_DrawParticles|QMB_DrawParticles (those 2 functions spawns particles)
This function runs particle physics:
Code: Select all
void Classic_DrawParticles (void)
{
particle_t *p, *kill;
int i, j;
float grav, time1, time2, time3, dvel, frametime;
unsigned char *at, theAlpha;
vec3_t up, right;
float dist, scale, r_partscale;
if (!active_particles)
return;
r_partscale = 0.004 * tan(r_refdef.fov_x * (M_PI / 180) * 0.5f);
GL_Bind (particletexture);
glEnable (GL_BLEND);
if (!gl_solidparticles.value)
glDepthMask (GL_FALSE);
glTexEnvf (GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE);
glBegin (GL_TRIANGLES);
VectorScale (vup, 1.5, up);
VectorScale (vright, 1.5, right);
frametime = fabs(cl.ctime - cl.oldtime);
if (cl.paused) // joe: pace from FuhQuake
frametime = 0;
time3 = frametime * 15;
time2 = frametime * 10; // 15;
time1 = frametime * 5;
grav = frametime * sv_gravity.value * 0.05;
dvel = 4 * frametime;
for ( ; ; )
{
kill = active_particles;
if (kill && PARTICLE_INACTIVE(kill))
{
active_particles = kill->next;
kill->next = free_particles;
free_particles = kill;
continue;
}
break;
}
for (p = active_particles, j=0 ; p ; p = p->next)
{
for ( ; ; )
{
kill = p->next;
if (kill && PARTICLE_INACTIVE(kill))
{
p->next = kill->next;
kill->next = free_particles;
free_particles = kill;
continue;
}
break;
}
// JDH (from BJP): Improve sound when many particles
if (++j % 8192 == 0)
S_ExtraUpdateTime ();
// hack a scale up to keep particles from disapearing
dist = (p->org[0] - r_origin[0])*vpn[0] + (p->org[1] - r_origin[1])*vpn[1] + (p->org[2] - r_origin[2])*vpn[2];
scale = 1 + dist * r_partscale;
at = (byte *)&d_8to24table[(int)p->color];
theAlpha = (p->type == pt_fire) ? 255 * (6 - p->ramp) / 6 : 255;
glColor4ub (at[0], at[1], at[2], theAlpha);
glTexCoord2f (0, 0);
glVertex3fv (p->org);
glTexCoord2f (1, 0);
glVertex3f (p->org[0] + up[0]*scale, p->org[1] + up[1]*scale, p->org[2] + up[2]*scale);
glTexCoord2f (0, 1);
glVertex3f (p->org[0] + right[0]*scale, p->org[1] + right[1]*scale, p->org[2] + right[2]*scale);
p->org[0] += p->vel[0] * frametime;
p->org[1] += p->vel[1] * frametime;
p->org[2] += p->vel[2] * frametime;
switch (p->type)
{
case pt_static:
break;
case pt_fire:
p->ramp += time1;
if (p->ramp >= 6)
p->endtime = -1;
else
p->color = ramp3[(int)p->ramp];
p->vel[2] += grav;
break;
case pt_explode:
p->ramp += time2;
if (p->ramp >= 8)
p->endtime = -1;
else
p->color = ramp1[(int)p->ramp];
for (i=0 ; i<3 ; i++)
p->vel[i] += p->vel[i]*dvel;
p->vel[2] -= grav * 30;
break;
case pt_explode2:
p->ramp += time3;
if (p->ramp >= 8)
p->endtime = -1;
else
p->color = ramp2[(int)p->ramp];
for (i=0 ; i<3 ; i++)
p->vel[i] -= p->vel[i]*frametime;
p->vel[2] -= grav * 30;
break;
case pt_blob:
for (i=0 ; i<3 ; i++)
p->vel[i] += p->vel[i]*dvel;
p->vel[2] -= grav;
break;
case pt_blob2:
for (i=0 ; i<2 ; i++)
p->vel[i] -= p->vel[i]*dvel;
p->vel[2] -= grav;
break;
case pt_grav:
case pt_slowgrav:
p->vel[2] -= grav;
break;
#ifndef _WIN32
default:
break; // shut up compiler warnings
#endif
}
}
glEnd ();
glDisable (GL_BLEND);
glDepthMask (GL_TRUE);
glTexEnvf (GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_REPLACE);
glColor3f (1, 1, 1);
}
The night is young. How else can I annoy the world before sunsrise? Inquisitive minds want to know ! And if they don't -- well like that ever has stopped me before ..
-
- Posts: 22
- Joined: Mon Apr 28, 2014 8:34 pm
- Location: San Carlos, CA
- Contact:
Re: cl_visedicts: are duplicate entries OK/expected?
Thanks for poking around at this some more.
So, looking at the patch above:
That will stop this particular lockup because it will avoid adding to cl_visedicts in the case where SCR_UpdateScreen is called from SCR_BeginLoadingPlaque.
I'm still concerned about all the other spots where SCR_UpdateScreen is called though... seems like those can also cause dups. So I'm wondering if there is a more general solution. (Or something that I'm missing about why a more general solution isn't needed.)
So, looking at the patch above:
Code: Select all
void CL_LinkEntity(entity_t *ent)
{
extern qboolean scr_drawloading; // Baker added
if (scr_drawloading) // Baker added
return; // Baker added
if (cl_numvisedicts < MAX_VISEDICTS)
{
cl_visedicts[cl_numvisedicts++] = ent;
}
}
I'm still concerned about all the other spots where SCR_UpdateScreen is called though... seems like those can also cause dups. So I'm wondering if there is a more general solution. (Or something that I'm missing about why a more general solution isn't needed.)
Re: cl_visedicts: are duplicate entries OK/expected?
Have you figured out why it crashes in particular? Sounds like you know where it is crashing, but what is it doing when it crashes -- and why?Johnny Law wrote:I'm still concerned about all the other spots where SCR_UpdateScreen is called though... seems like those can also cause dups. So I'm wondering if there is a more general solution. (Or something that I'm missing about why a more general solution isn't needed.)
Does the engine hardly ever crash during gameplay? Or does it crash with some frequency?
The night is young. How else can I annoy the world before sunsrise? Inquisitive minds want to know ! And if they don't -- well like that ever has stopped me before ..