Have a look at how W_CheckNoAmmo looks in the standard Quake code, the version in your mod isn't the same. Even the original W_CheckNoAmmo function is arguably badly designed: you might expect that it's just a function that returns TRUE if the current weapon is out of ammo, and FALSE otherwise. Instead it:
a) returns TRUE if the weapon has ammo and FALSE if it doesn't
b) has a side-effect - if you had no ammo, it changes you to a weapon you can attack with!

In the original code there's an exception that prevents it changing your weapon if you are attacking with the Axe. This makes sense - the axe doesn't require any ammo, so there's no need to switch away. In your mod the axe is replaced by items which do have ammo counts, you can't just wave through axe-slot-weapons completely unchecked. But you need to add a more specific exception for the situation you describe - if the player has the grappling hook out and is currently suspended from a rope, then return TRUE before the W_BestWeapon part. 

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?