Touch links

[Baker]

FitzQuake 0.85 has a touch links fix that apparently breaks the "White Room" mod. Quakespasm has the "right" solution according to what I've read.

I'm interested in this problem except ... I don't understand the problem at all. So I don't even get the issue, any aspects of the problem or why it works or doesn't work or what it is doing so I don't know what these fixes are looking to fix or why one works and another one doesn't.

is that I'd like to implement Quakespasm's fix for this, but can't even describe the problem nor do I know really what it is fixing and can't develop a test case.

Short version: wats server touchlinks do? how is this a problem and when does it happen and how does Quakespasm's fix solve that and metlslime's fix solves it sometimes but I guess can also break it sometimes? Or is White Room doing something "bad"?

[Spike]

SV_LinkEdict links the entity into the world.
basically there's a small bsp tree (constructed in a grid like an octtree, I don't remember the technical name) that it uses to track which entities are where
each area has two lists. one for solid ents and one for triggers.

SV_TouchLinks:
walks through the nodes that are within the area stated. for each area, walk through the 'trigger' list and invoke the touch function on each.

here's the bug that you're probably refering to:
the touch function inside the trigger list invokes touch functions. if the touch function moves the entities around, they will call SV_(Un)LinkEdict. this will change the links that SV_TouchLinks is currently walking through.
This will result in infinite loops and crashes and stuff.

to reproduce in vanilla:
map e2m2
shoot one button
fly/rocketjump/whatever over the bridge that isn't there yet.
this will result in a killtarget remove() of the trigger_once that you just walked through.
the qc will then return to the engine and back to sv_touchlinks. touchlinks will try to continue walking the list which is now set to NULL or something... CRASH!

the q2 solution to this is to build a list of all the ents that are within the box, then to invoke touch functions on each found. you do need to make sure its not removed by a prior touch function though, but the spawn() deadtime of 2 secs avoids any remove()+spawn() issues.
just validating the links within SV_TouchLinks is probably not 100% correct, and would be slow if it was.

[szo]

QuakeSpasm currently uses Bill Currie's solution from QuakeForge. Here is the relevant QF commit:
http://quake.git.sourceforge.net/git/gi ... c7136d97b7

[r00k]

Hmm this was an old bug, I thought MH or Spike posted a fix, I'll have to search my source but I cannot reproduce this bug in Qrack, and the white room works.

[szo]

[r00k]

[mh]

The criminal struct is called "areanodes", and is used to store which edicts are in reasonably close proximity, so that collision and other interaction tests can be restricted to sub-groups of edicts rather than needing to exponentially increase as edict numbers increase. It's a basic "divide-and-conquer" strategy (for the curious there is optimization potential in this for big maps - just create more areanodes).

The bug happens because SV_TouchLinks does a linear walk through the edicts in a node, and as part of that walk will call PR_ExecuteProgram if entities touch. On occasion this call to PR_ExecuteProgram may free an edict that has yet to be hit in this walk, or may even relink the edicts in that node, meaning that it can create infinite loops or dereference NULL pointers.

I'm not too familiar with the QS fix; my fix is to copy off the list of edicts that touch to scratch memory during the initial linear walk, then run the touch functions for any edicts that I copied off during a second pass. That works because nothing is modified during the critical first pass.

[taniwha]

Heh, I very vaguely remember that fix (ie, on seeing the patch, it feels familiar). It's nice to know that my fix is holding up well

I wonder what caused me to fix it.

[taniwha]

Heh, after having that patch pointed out, I got to worrying about it.

sv_link_prev is never set (not really a problem). I guess I was thinking ahead to when something might be doing reverse searches.

sv_link_next points to the local "next" variable, thus next gets updated as necessary.

However, there does seem to be a potential problem: if an entity is moved, it will be re-linked. This might cause an entity to be touched twice (or even an infinite loop). I need to check further.

[Baker]

Thanks for all the various info everyone. I'm reading it right now, but I don't have anything smart to say except (@Spike yeah ... E2M2 crash is familiar).

LOL @ MH "The criminal struct ..." .

More: Ok ... with all the descriptives above of what is causing the issue and how it arises, still not familiar with areanodes but I can change that.

LOL #2 @ R00k: Has fix and didn't know it plus didn't remember doing it. Easy enough to relate to that.

[r00k]