Fix shotgun pattern origin in g_weapon.c

[scoqx]

Make server-side start point the same as client's prediction

baseq3a/code/cgame/cg_weapons.c

Line 2222 in e24c841

CG_ShotgunPattern( es->pos.trBase, es->origin2, es->eventParm, es->otherEntityNum );

[WofWca]

Great catch! Indeed the pattern is off. Sometimes it's possible to get the "blood" effect but no damage (especially easy to reproduce if you set fixed seed, tent->s.eventParm = 255):

However, see my comment.

I suspect that this piece of code is also affected? Although it's only about Team Arena.

baseq3a/code/game/g_weapon.c

Line 666 in 5a4ab3a

tent = G_TempEntity( muzzle, EV_LIGHTNINGBOLT );

[WofWca]

I think it's better to make the client-side stuff match the server side, rather than the other way around. The latter actually changes gameplay, the former does not. So, keep ShotgunPattern as is and instead change the origin of G_TempEntity to muzzle_origin.

Suggested change

	ShotgunPattern( tent->s.pos.trBase, tent->s.origin2, tent->s.eventParm, ent );
	tent = G_TempEntity( muzzle_origin, EV_SHOTGUN );
	VectorScale( forward, 4096.0, tent->s.origin2 );
	SnapVector( tent->s.origin2 );
	tent->s.eventParm = rand() & 255; // seed for spread pattern
	tent->s.otherEntityNum = ent->s.number;
	ShotgunPattern( muzzle_origin, tent->s.origin2, tent->s.eventParm, ent );

See the following comment. Looks like muzzle_origin is a better match for hitscan traces. Also it's used for all other weapons.

baseq3a/code/game/g_weapon.c

Lines 10 to 11 in 5a4ab3a

	static vec3_t muzzle;
	static vec3_t muzzle_origin; // for hitscan weapon trace

[WofWca]

Ah, but my suggestion, however, would still result in a bit of inaccuracy, due to snapping:

baseq3a/code/game/g_utils.c

Lines 478 to 490 in 5a4ab3a

	gentity_t *G_TempEntity( vec3_t origin, int event ) {
	gentity_t *e;
	vec3_t snapped;
	e = G_Spawn();
	e->s.eType = ET_EVENTS + event;
	e->classname = "tempEntity";
	e->eventTime = level.time;
	e->freeAfterEvent = qtrue;
	VectorCopy( origin, snapped );
	SnapVector( snapped ); // save network bandwidth

But I think it's acceptable.

[scoqx]

I think it's better to make the client-side stuff match the server side, rather than the other way around. The latter actually changes gameplay, the former does not. So, keep ShotgunPattern as is and instead change the origin of G_TempEntity to muzzle_origin.

See the following comment. Looks like muzzle_origin is a better match for hitscan traces. Also it's used for all other weapons.

baseq3a/code/game/g_weapon.c

Lines 10 to 11 in 5a4ab3a

	static vec3_t muzzle;
	static vec3_t muzzle_origin; // for hitscan weapon trace

I believe it's better to make the server match the client in this case. For over 20 years, everyone has been using the client-side implementation (even OSP 1.03, which was confirmed via decompilation). Forcing everyone to update their clients is not the right approach here.

Every mod I have checked contains this exact same fix (or bug). They all share one thing in common: the client calculates it from s.pos.trBase rather than muzzle. Therefore, I believe the correct approach is to fix the server-side code

[WofWca]

Forcing everyone to update their clients is not the right approach here.

I'm not suggesting to touch client code. I am saying that we need to change the origin of the EV_SHOTGUN temp entity to match the origin passed to ShotgunPattern, and not the other way around (changing the origin passed to ShotgunPattern to match the origin of the temp entity).

Every mod I have checked contains this exact same fix (or bug)

That's how things are in the original Quake III source code. See my comment below, https://github.com/ec-/baseq3a/pull/78#issuecomment-4854832943

[scoqx]

Forcing everyone to update their clients is not the right approach here.

I'm not suggesting to touch client code. I am saying that we need to change the origin of the EV_SHOTGUN temp entity to match the origin passed to ShotgunPattern, and not the other way around (changing the origin passed to ShotgunPattern to match the origin of the temp entity).

Every mod I have checked contains this exact same fix (or bug)

That's how things are in the original Quake III source code. See my comment below, https://github.com/ec-/baseq3a/pull/78#issuecomment-4854832943

Sorry my bad

[WofWca]

Doing more digging: muzzle_origin has been introduced in 9a7c907, so this commit would partially revert that (unless you apply my suggestion).

[scoqx]

Devotion

https://github.com/alcachofass/devotion/blob/f9172d673313815209fccbe0c68e040411c6cc3a/ratoa_gamecode/code/game/g_weapon.c#L589

OA

https://github.com/OpenArena/gamecode/blob/5478aad23b12857d265103f6aa2f5258c78799c8/code/game/g_weapon.c#L455