Yeah, the way axe replacements are handled is indeed annoying. It shouldn't switch directly to the best weapon if you run out of hooks/flares/TNT. But can this completely be solved in weapons.qc or would you have to change stuff in other files, too? 

It's a starting point; try that first, add some dprint statements so you can check whether your changes are correctly coded and do what you expect. If it still doesn't fix the problem, then you can start expanding the search. As Raymond Chen says, "debugging is an exercise in optimism". 

I am afraid I am still not able to get it working. Neither the code insertion point(s) nor what is actually supposed to be written there is clear.

Maybe the check already needs to happen while firing. If I want to hide the flare model when ammo counter reaches zero, it's only hidden after I press the fire button again after the counter is already zero. However, this should happen right after firing the last available flare. We are not speaking of the grappling hook, which is a special case and I still have no clue how to tell the game it's supposed to detach you still from the chain after firing the last hook and ammo counter has reached zero. 

for the grappling hook, a possible solution would be to subtract the ammo on detatch rather than on launch. That way you would still have 1 ammo when you are attached to the final rope. Not sure if this would be considered confusing to the user, to see 1 ammo when they are currently hanging by the last rope. But as soon as they let go and it went to zero, they would probably understand. 

Actually I managed to make flares and TNT behave the way I wanted. If their counter goes below 1, it directly switches to the next available tool (and not the best weapon) instead of reaching zero. Best weapon is only selected if all tools are empty. To achieve this, a small addition had to be made in W_Attack of weapons.qc (where the axe is defined, replaced by the utility vest).

Right now, the grappling hook already has the intended behavior of counting down to 0 when you are dangling from the final chain. However, pressing Attack again would then switch to the best weapon instead of detaching the player. You can still get off the hook by jumping, but it's not as intuitive as it could be. 

What does the code that makes the player detach when they jump look like? Can you copy that into W_CheckNoAmmo? 

Tbh I don't know why jump is detaching you from the chain in general. However, I realized now that you somehow must get into the firing function once again after you ran out of hook ammo.

This means something really needs to happen in W_CheckNoAmmo. You have to get back into the hook firing state with zero ammo and there you need a condition for exactly this situation, just detaching you from the hook and that's it. 

I think finding the jump code would be telling. My hope is that there's a function called something like HookDetatch() which is what you need to add to W_CheckNoAmmo, but it depends how the person writing the original code did it. They might have chosen to just add the code that detaches the player "inline" - copy-pasting it rather than splitting it out into a useful function.

I would start the search for the code in PlayerJump in client.qc. You might want to have PlayerJump from the original ID1 source available for reference, so that any added lines of code will stand out. If that doesn't yield anything, it might be that some of piece of code is checking for the jump button directly. I'd search for references to "button2" as this is the field which contains the jump button status - but you'd have to search the entire code for it. 

How about a hackish approach: Simply let the player jump via attack button if hook ammo is zero and player is still attached to the hook.

Could be as simple as
if (self.button0) { PlayerJump (); }

Basically that's what is supposed to happen right now, but at least you would keep using the attack key. Might this work? 

It seems the idea was a good one. I was able to find the jump function in client.qc and got inspired.

The working code part for weapons.qc looks as follows:

void () W_FireChain;

float () W_CheckNoAmmo =
{
if (self.currentammo > DROIDMODE_GRAP)
{
return (TRUE);
}
if ((self.grap_out == GRAP_STUCK) && (self.button0))
{
W_FireChain ();
W_ToggleDroid ();
}
else if (self.weapon != IT_AXE)
{
self.weapon = W_BestWeapon ();
}
W_SetCurrentAmmo ();
return (FALSE);
};

Works like a charm. Thanks to everybody who gave useful hints! 

I have posted my finished Shrak patch over at the QuakeOne forums, in case anybody is interested. Source files are included. 

Well done Nightfright!

Would you be so kind to publish a working shark.qc? 

If you mean the source code, it's included with the patch in a separate dir. I can upload that separately somewhere (if I knew where). 

See the screenshot thread. 

Comes with documentation about applied changes:
Download (183 KB) 

FYI, I created a news thread for your patch. 

Good job!I wonder how that Gorok enfolds. 

It seems there is another problem in the Shrak code, something I didn't know about until it was pointed out to me in the QuakeOne forums.

If you use the nextweapon or lastweapon keys really fast, the game will crash. It will already happen in the start map when you just have the default weapon. Apparently won't happen if you have all weapons plus the full utility vest with all three items.

Example for the error:
EQ_F 4226 (?) 241 (FL_JUMPRELEASED)4096.0 4226(?)

weapons.qc: CycleWeaponCommand
weapons.qc: ImpulseCommands
weapons.qc: W_WeaponFrame
client.qc: PlayerPostThink

(No Function)
Runaway Loop Error
Host_Error: Program Error 

It was something in ImpulseCommands, as I suspected right away. An abort/delay command was placed too far down in the code. Placing it in the beginning prevents the crashes from happening. 

Still, it feels like a workaround since there is a delay while switching weapons while original Quake does not have it. If you remove the delay entirely or reduce the delay too much (below self.dlayTimer = time+0.1 in client.qc/PlayerPostThink), crashes will reappear again when switching weapons too fast. 

I never use switch weapons, only the number keys.

A big Hurah for the four stars encountered: DOE.qc - SOA.qc - Malice.qc - Shrak.qc !

Nightfright, you rock! 

The Friend Maker (friend.qc) used to shoot a cloud of yellow pixels, but when using this patch, it does not appear. Couldn't find it anywhere in the code. 

It seems instead of
missile.effects = EF_BRIGHTFIELD;
I had to put this in friend.qc:
missile.effects = 1;

No idea why it doesn't work with the actual name, but main thing is it works. 

I believe I have tracked down the origin of the crashes when switching Shrak weapons.

Whenever W_ChangeWeapon or one of the two weapon cycling (impulse 10/12) functions is called, it's flashing the weapon in the HUD, something like flashWeapon (fl) or flashWeapon (self.weapon). If I comment out this line, crashes won't happen any more regardless of mousewheel scrolling speed.

The function is as follows:

void () flashWeaponBack =
{
if (!(self.owner.items & self.items))
{
self.owner.items = (self.owner.items + self.items);
}
remove (self);
};

void (float fl) flashWeapon =
{
local entity e;

e = spawn ();
e.owner = self;
e.items = fl;
e.think = flashWeaponBack;
e.nextthink = (time + 0.1);
if (self.items & fl)
self.items = (self.items - fl);
};

Can anybody see what could be wrong with this? 

OK, I think I see the shape of the problem if not the exact fix, brace yourself...

The crash you posted a little while back contains the error message indicating there's an infinite loop going on. CycleWeaponCommand is built around a potentially infinite loop:

while (1)
{
...
}

The only escape from the loop is the return statement near the bottom

if ((self.items & self.weapon) && (am == DROIDMODE_GRAP))
{
flashWeapon (self.weapon);
W_SetCurrentAmmo ();
return;
}

The first run through the loop looks at your current weapon and "suggests" changing to the next weapon in the game after that. This final bit of code checks whether you have the suggested weapon and ammo to use it; if you do the code changes to it and exits the loop. If not, the next run through the loop starts from the suggested weapon, and changes the suggestion to the weapon after that.

In regular Quake, there is an implicit guarantee that this loop will terminate: the player always has the axe, and the axe is always a valid weapon to select. The loop will suggest the axe after at most 8 attempts, the axe is always possessed by the player, and there's never an ammo requirement for wielding it.

In the Shrak code, the weapon of last resort turns out to be in slot 2 (IT_SHOTGUN). This is the only weapon which doesn't have an ammo requirement. This would be OK ordinarily, and it's only when we consider the effect of flashWeapon that we get to the problem.

flashWeapon makes the icons flash on the hud in a rather dramatic fashion. It genuinely removes the weapon from the player's inventory for a fraction of a second, before putting it back. If you've followed what's going on in the description of the loop above, perhaps you can see the problem that arises...

Because the weapon is actually removed from the player inventory, however briefly, there exists a window where CycleWeaponCommand can't select that weapon and has to skip it. If you quickly flash all of the player's weapons at once (maybe cycling weapons rapidly with the scroll-wheel) all of the players weapons can be removed at once. At this point, CycleWeaponCommand has no suitable candidates to select, but it doesn't give up, it just keeps trying all the weapons in loop after loop until the game crashes.