Can you go back and undo the white space changes so the diffs only contain what you’ve added? You’ve really nuked line endings here

Is TR_ClipRayToEntity ever useful outside of the TR_EnumerateEntities callback?

Does something like the following make more sense to be consistent with the other TR natives:

public Action Command_TriggerTest( int client, int args )
{
	float startpos[3];
	GetClientEyePosition( client, startpos );
	float angles[3];
	GetClientEyeAngles( client, angles );
	
	// native void TR_EnumerateEntities(const float pos[3], const float vec[3], bool triggers, RayType rtype, TraceEntityEnumerator enumerator, any data=0);
	TR_EnumerateEntities( startpos, angles, true, RayType_Infinite, HitTrigger );

	// TR_EnumerateEntities internally calls TR_ClipRayToEntity before returning when enumerator returns false.
	//TR_ClipRayToEntity( MASK_ALL, entity );
		
	float pos[3];
	TR_GetEndPosition( pos );
		
	PrintToChatAll( "Hit a trigger! Collided at (%.2f, %.2f, %.2f)", pos[0], pos[1], pos[2] );
}

// return true to continue enumerating, or false to stop
public bool HitTrigger( int entity )
{
	char classname[64];
	GetEntityClassname( entity, classname, sizeof( classname ) );	
	return StrContains( classname, "trigger" ) == -1;
}

This is making the assumption that after you've clipped the ray you'll always be returning false, is this assumption correct?

When is the mask param to TR_ClipRayToEntity needed? (other than MASK_ALL)

If TR_ClipRayToEntity is needed, I think it should probably be used with TR_GetEntityIndex, rather than inside the enumerator.

Only other thing I would like to see is replacing the boolean triggers param with a flag param - we're trying to move away from non-obvious boolean params as they make code harder to understand quickly.

(I had a different post previously but realized something at the last minute and have revised it)

I think the best solution is to call ClipRayToEntity automatically as you suggest (assuming it is not too wasteful if the user doesn't actually need it), and to also make it directly available, but with full ray parameters like this:

native void TR_ClipRayToEntity(const float pos[3], const float vec[3], int mask, RayType rtype, int entity);

This also makes the mask parameter make more sense.

I definitely think ClipRayToEntity should be available directly. I can envision scenarios where you have a known entity and just want to clip a ray to it, and having to go through EnumerateEntities would be odd.

At first I was against calling ClipRayToEntity automatically because it could potentially be wasteful to do so, but if we want to allow calling ClipRayToEntity on its own without it being tied to another TR_ call, then ClipRayToEntity needs its own ray parameters which would make it very awkward to manually call within or in response to the EnumerateEntities enumerator.

Edit: Also need to make sure ClipRayToEntity causes TR_DidHit to return true.

This is making the assumption that after you've clipped the ray you'll always be returning false, is this assumption correct?

Clipping the ray doesn't always mean that you want to stop enumerating. The condition for stopping the enumerating could be dependent on the results of ClipRayToEntity (or something else entirely).
e.g. You only want to stop enumerating if the entity position meets some requirements. I think it would be best if its left up to the people using these functions instead of taking away that option.

When is the mask param to TR_ClipRayToEntity needed? (other than MASK_ALL)

This depends on the use case. Using different masks on entities could give different results depending on where it hits them. For example, say I wanted to clip the ray to a players bounding box rather than it's hitbox, I could use MASK_PLAYERSOLID for that. I tested this out myself using this code. Here's an image showcasing it as well (red hits hitbox, green hits bbox): https://i.imgur.com/HjNZqEy.jpg

Only other thing I would like to see is replacing the boolean triggers param with a flag param

Do you mean making an enum specifically for this? I only made it a boolean since the function it calls also uses a bool so I thought that would be the most straightforward way to do it.

Also need to make sure ClipRayToEntity causes TR_DidHit to return true.

When the clipping doesn't hit anything, the trace result fraction is set to 1.0 and DidHit returns false

Also I wasn't sure about my ClipRayToEntity implementation, since it creates a new trace result out of the global ray. When you want to use the global trace result you pass in INVALID_HANDLE, or pass in a handle and it gets filled with a new trace result. Should I change this behaviour to work like the other TR functions with a regular/Ex version?

I've pushed another commit that seperates ClipRayToEntity into 2 groups of natives:

• TR_ClipRayToEntity(Ex) - This now takes a start/end/raytype like the other Trace natives and initializes a new ray before calling ClipRayToEntity.
• TR_ClipCurrRayToEntity(Ex) - This works like the implementation before this commit where it just performs ClipRayToEntity on the current global ray.

This way the ClipRayToEntity function isn't tied down to EnumerateEntities and can also be used by itself.
I also split those natives into regular/extended versions that return new trace results to make them more like the other natives.

Here is the same example plugin I posted above but updated to use the new natives: https://pastebin.com/CB5uJFLz

Any thoughts?

That implementation looks solid in my opinion.

Just to clarify why SildyBat went with two versions of ClipRayToEntity (not including their Ex counterparts):

At a minimum, it makes sense to have a version of ClipRayToEntity that mirrors the original (it accepts an arbitrary ray definition); but there is also potential for a need to call ClipRayToEntity as part of the EnumerateEntities enumerator - especially since the two tend to go together. Since we don't have access to the ray/trace instances in SourcePawn, having TR_ClipCurrRayToEntity makes that much easier. Without it we would need to pass something like a datapack to the enumerator and repeatedly initialize a new copy of the same ray.

Can we have this please?

Yeah please.