A_Face* pitch_offset miscalculation

[Major Cooke]

When I tested this before submitting, turns out I accidentally did it with the wrong parameters organized. This is why I didn't catch it sooner. -_-;

The pitch_offset works as intended with the FAF_NODISTFACTOR, but without it, it's grossly wrong. I'm working to fix it now but its proving more difficult than I thought...

All I can tell is, in p_enemy.cpp,

Code: Select all

if (!(flags & FAF_NODISTFACTOR))
			target_z += pitch_offset; 

This must not be there. Instead, I think it belongs somewhere in:

Code: Select all

double dist_z = target_z - source_z;
		double dist = sqrt(dist_x*dist_x + dist_y*dist_y + dist_z*dist_z);
		int other_pitch = (int)rad2bam(asin(dist_z / dist));

But I'm having a bit of trouble figuring it out...

The point behind it was to make it factor distance with pitch offset, and if possible, angle too. (Unfortunately I didn't know how to deal with the angle.)

[Major Cooke]

I tried taking another crack at this but I'm pretty stumped. Anybody have any ideas?

[Chilly]

Hi.

I've had a look at the code and the explanation on the Wiki and I'm still not sure what's going on. It's clearly wrong because here

Code: Select all

if (!(flags & FAF_NODISTFACTOR))
         target_z += pitch_offset;

you're adding an angle to a length.

1. Could you explain exactly what FAF_NODISTFACTOR should do. Preferably with an example.

2. I saw on the Wiki for A_FaceTarget, "This is exactly like calling A_SetPitch after this function, where the raw value of pitch_offset is unfiltered by distance." It appears that A_SetPitch sets the pitch to a specific value whereas this function merely offsets the pitch, i.e. the Wiki suggests A_SetPitch(offset) but the code suggests A_SetPitch(pitch+offset). Which is it?

[Major Cooke]

FAF_NODISTFACTOR works just fine. It does just as the code suggests. It adds the offset onto it. Works like performing A_SetPitch(pitch_after_facing_target+pitch_offset)

It's without the flag that there's an issue. And, again, I don't know how to fix it, and neither does anyone else from the looks of it.

[randi]

I have no idea what your intent was here. What is "DISTFACTOR"? I'm inclined to remove the flag altogether and use the path you set if it's present since it makes no sense.

[Major Cooke]

The idea was, I wanted to make an actor that first aims at the feet of an actor, then aims, say... +50 units up the target to its head. WIth distance factoring, up close to the actor. The further away the target is, the less pitch it offsets.

And then FAF_NODISTFACTOR means just add the raw pitch offset as it is specified, not as part of the distance calculation.

The green line here represents A_Face* without pitch offset added. The yellow bar represents adding 50 offset to the calculation of distance.

[Graf Zahl]

I'm inclined to remove the flag altogether and use the path you set if it's present since it makes no sense.

I see what he wants here, and Chilly hit the nail with his remark 'You are adding an angle to length.' In short: Both variables have entirely different value ranges and the code as it is has to be considered broken by design.

The flag indeed makes no sense as it mixes two things that should actually be separate parameters because they are separate types.

- an offset to the pitch (i.e. it actually changes the calculated pitch, type = angle_t)
- an offset to the target's position. (i.e. it changes the position to look at, type = fixed_t)

[Major Cooke]

I've fixed it.

[Graf Zahl]

No, sorry. You can't just remove the flag altogether. The DECORATE definition needs to stay and the value may not be recycled for other purposes. The function has been around for too long to do that.

[Major Cooke]

I left it in the constants.txt file. What would you have me do specifically with the flag internally though?

[Graf Zahl]

Nothing.

Another thing: I'm not convinced that what you do with FAF_MULTIPLY is mathematically sound.

[Major Cooke]

Alright done. Values restored. And here's a test package to try it out. As usual, summon MCD. Now he fires his rockets with three different settings.

[Graf Zahl]

I decided to do this one myself, without FAF_MULTIPLY and use the opportunity to clean this up a bit.