If you want to stop the queueing, then putting that the return in the if() in W_WeaponFrame would do it. If however, you want some impulses to always happen, then you need to put a separate check in, to happen before that check.

Of course, an even more intricate fix (which goes in the other direction from where you want to go) is to track what the impulses would make the weapon change to if you were not firing (setting .weapon appropriately, so the hud light shows what you will use next), and then switch to it when the player has stopped firing. Of course, there is another issue again, in that I don't think you can switch away while firing a nailgun, because the way they fire is handled differently is handled to other weapons. 

i just moved the self.impulse = 0 to the end of W_WeaponFrame and removed the if check with the return. i have no idea where the if check in ImpulseCommands came from but i took it out as well and it works fine after re-arranging the way firing and impulsecommands was handled. 

You should probably use else if in ImpusleCommands(), so that it can go faster, May not make that much difference these days, but it's probably a good idea to anyway.

Even more of a timesaver, though, is skipping all of the impulse checks completely if self.impulse is 0. I don't actually know when impulse gets truncated to the lower 8 bits, but I think it's before the QC sees it (and possibly it only gets sent over the network as a byte), so you don't have to check for it being anything else that resolves to 0. 

There's a built in way to check how fast qc runs on an engine, in case you're optimising things. The command is "profile", and it lists the top 10 functions in terms of commands executed since the last profile command or server start. As an example on unpatched quake:
e1m6 at 72fps
425616 PlayerPreThink
270814 WaterMove
241542 PlayerPostThink
202689 ImpulseCommands
177480 CheckRules
165648 CheckPowerups
125843 ai_stand
94928 StartFrame
76144 door_touch

Things worth noticing:

*ImpulseCommands is certainly up there. Almost certainly the most important optimisation would be to skip everything if self.impulse is 0. I'd agree with doing that check before calling the function ImpulseCommands(calling functions costs a few operations).
Further optimisations would be the else if Lardarse suggests, and storing self.impulse in a local float at the start of the ImpulseCommands function - to avoid looking it up each time, which is more expensive if you have to go through a layer of self.something first. But optimising the very rare cases where there is a change of weapon isn't gonna yield anything like the benefit of skipping the whole thing in 99.9% of frames.

* CheckRules places highly, even though that function is entirely pointless in single player games. I think the calls to cvar are pretty expensive here.

* Results from this kind of assessment will be different at different framerates. The playerprethink type functions occur every frame, but since monsters think in 0.1 second slices, that's only 1 in 7 frames. Playing at 15fps, I'd imagine ai_stand could top the charts. 

I've learned that if you create a new monster with a new model, the $frames still have to be named in the progs.dat urnewmonster.qc file in the same order they're in the .mdl.

I assume the compiler converts frame names to integer offsets and the names of the frames themselves are irrelevant? 

I assume the compiler converts frame names to integer offsets and the names of the frames themselves are irrelevant?
yup. 

Is it relevant in any way how many frames there are per line? I've had no problems, but then again I haven't experimented much ie. tried to break it.

Is there anything stopping me from having, say, 20 frames on the same line in the .qc? 

and it didn't seem to mind 

whenever i coded new monsters, i just looked up the frame number in QME and just typed the number, rather than having to list all the $frame bollox at the top of the file 

isn't that arguably more confusing? if I reexport the model with new anims in it it's kind of nice to just be able to insert them in the list at the top than have to replace all of the numbers in all of the frame functions ... 

If you're thinking about an end user for the code that's a nightmare. Also depends on having the anims in one seqeunce rather than seperated.

It's interesting hacking up all this stuff. 

and storing self.impulse in a local float at the start of the ImpulseCommands function
Probably not worth it. Except maybe for RuneQuake, or anything else that has impulse overloading...

The frame macros can be called anything you like. It's usual, though, to call them something similar to the model, as it makes using them easier. And as how how many you can fit on one line, I think your most sensible limit is the right edge of your text editor... 

Yeah, I agree it's not useful in ImpulseCommands once you have the other improvements. But once you sort out the needlessly expensive functions like that, you start to notice that functions like visible() and range() begin to show up in profile. And in those, you do get (theoretical) performance improvements by using locals. This is my current optimised range function

float(entity targ) range =
{
local vector spot1;
local float r;

spot1 = self.origin + self.view_ofs - targ.origin - targ.view_ofs;
//PREACH: this only uses two temps
r = spot1 * spot1;
//
//PREACH: kept in a local since we use it twice
//PREACH: it would still be stored in a temp local float
//PREACH: but would re-evaluate each time in case spot1 had changed. We know it can't, so we save the result.
if (r < 500 * 500)
{
if (r < 120 * 120)
return RANGE_MELEE;
else
return RANGE_NEAR;
//PREACH: nesting the ifs like this takes at most 2 comparisons, down from 4 in the original
}
else
{
if (r < 1000 * 1000)
return RANGE_MID;
return RANGE_FAR;
}
}

Skips the expensive calls to vlen, and nests the ifs for a slightly shorter path to RANGE_MID or RANGE_FAR. Note that in frikqcc and fteqcc with the flatten constants optimisation on, 120 * 120 and the others are evaluated at compile time. If you're not using that optimisation, then you should work out those values ahead of time. I leave them like that to make clear where the magic numbers come from.

You can get a little more juice out of visible if you use

float (entity targ) visible =
{
traceline (self.origin + self.view_ofs, targ.origin + targ.view_ofs, TRUE, self); // see through other monsters
if (trace_inwater)
{
if (trace_inopen)
return FALSE; // sight line crossed contents
}

if (trace_fraction == 1)
return TRUE;
return FALSE;
};

In FTE, putting the arguments straight into the traceline function call is actually faster than not doing so. I also put the trace_inwater check first, as I figure that's more likely to fail most of the time, so you don't need to bother checking trace_inopen(which under the same assumptions is usually likely to succeed). Doing && in an if statement always evaluates both statements in qc, so nesting ifs can make faster executing code where it matters. 

make the profile command show more than 10?

And of course, format things so they make sense to whoever is writing the code; for me this means not putting things on multiple lines for the hell of it :-) 

Isn't this the function that you suggested changing in another thread (can't find it) to make monsters see you while in water? 

I wouldn't say the water vision change was something that should always be changed, someone was asking how it could be done, so I posted it. Really it's a circumstantial choice, if it causes problems in a specific map then visible is the place to change things, but in general keeping water opaque is a good idea for fish if nobody else. 

Selective based on monster type, though. 

isn't that arguably more confusing?

hehe, well I admitted it for the comedy value really; I'm aware that it is an atrocious way of coding monsters :) 

Did I ask this before? I don't remember, honestly.

Anyway, is there a concept of a NULL value in QuakeC? Like say I want to define a local entity variable and set it to NULL before doing some other processing and at the end do a check like:

if( myEntity == NULL) or if( !myEntity )

Is that possible? I think I'm missing something obvious because I can't get the compiler to accept null, Null, NULL or 0 (zero).

Help? 

Most things tend to use world as the "NULL pointer". find() and variants usually return world if nothing is found, or end a linked list with world. I believe traceline() sets the hit entity to world if nothing is hit (the general way to detect no hit is if(trace_fraction == 1) although sometimes testing for hitting world is acceptable).

Usually, world is entity 0 (maybe eprint() might be able to tell you), although the compiler probably doesn't accept the implicit cast from float to entity.

Other NULL values are 0, "", and '0 0 0'. 

Just to add that

if( !myEntity )

is valid qc and equivalent to

if(myEntity == world)

If world can be a valid return type you need to deal with that as a seperate case somehow. 

Ahh alright. Thanks! 

so, say i have ent1.owner = ent2
and ent2 is removed. what happens to my pointer on ent1.owner? is it set to world or is it now corrupted junk?

can i do a check if (!ent1.owner) now and it'll pass? 

The entity field continues to point to the same "slot" that the original entity occupied. Worse, after two seconds that entity slot can be reallocated when a new spawn() request is received, so the field now points to a valid entity, but an entirely unrelated one.

It's also worth noting that most of the fields on the removed entity will still be readable, and contain the same values they did before the entity was removed. The engine resets a few fields on removal, like solid, model and origin, but doesn't zero all of them until it's reused by the spawn() command. You can observe a side effect in fitzquake if you turn r_showbboxes on; a removed entity creates a bbox of that entity's size at the origin.

You could exploit the above fact to write code which can tell if an entity is removed or not:
Before removing an entity, set a float .isremoved on that entity to TRUE. The value can still be read, and will be reset when the entity is respawned as something else. You have to pinkie-swear never to modify that value outside the remove() function for it to be reliable.

I wouldn't recommend actually doing that though, because it's kinda relying on "undefined" behaviour in the engine - that removed entities can still be read. Engines which had more advanced entity management might not enforce that.

It's probably better to have a destructor function for the entity which is being removed which knows which entities will refer to it and sets them to WORLD. This would be practical if it's a master-slave kind of thing, not so much if you're worried about a monster getting removed and other monsters suddenly having an invalid .enemy... 

"You can observe a side effect in fitzquake if you turn r_showbboxes on; a removed entity creates a bbox of that entity's size at the origin. "

Hahaha, so THAT'S what that is. I wondered what that box at the origin of the world was. Thanks! I thought my code was doing something wacky...