More Pitch Fixes

[Major Cooke]

Pull Request

After finally untangling my code and removing all pitch *= -1 and/or cos(-pitch) and sin(pitch) executions (from the old wiki edit), in compliance with the fact that cos(pitch) for vel.x and -sin(pitch) is the right way to go, I discovered four functions with inverted pitch semantics and fixed them accordingly.

Note: P_DrawRailTrail only makes use of pitch when spawning an actor, and P_RailAttack inverts it. That just reinverts it back to normal again so the spawned actors can have the proper pitch.

Here's D4D.pk3 for proof if testing is desired. The lightning gun and the BFG are two particular ones that had it bad but now work properly after adjusting their code, mod-side.

[Major Cooke]

Do these look alright? I would like to know so I can fix D4D without using bad pitch hacks. And yes, I would have to rely on them if I spawn them with different functions. I.e. I would have to make different actors specifically for A_FireCustomMissile and A_RailAttack if it's left as it is, but this fix will banish that requirement.

[Graf Zahl]

Later. I'm a bit sick today and really not in the mood to do some real work.

[Major Cooke]

Rest well!

[Matt]

So what's going on with pitches right now? It seems the newest versions have turned all the monsters' aiming upside-down...

EDIT: Just looked at the changelog. So we're basically just breaking all mods that used the old pitch calculations where everything that wasn't the player used positive pitch to mean up???

[Edward-san]

Vaecrius, which monsters?

In any case, too bad, MajorCooke. I think these pitch issues must be fixed with a new flag (for example let's call it CMF_DOOMPITCH for A_CustomMissile).

[Graf Zahl]

I reverted that change. And no, there won't be a flag.

[Graf Zahl]

For pitch fixes, the rule should be:

If the current behavior deviates from 2.8.0 or 2.7.0 it's a bug and needs fixing. This was the case with A_Face*.
If some function parameter is just negated, bad luck: It needs to remain that way.
My current solution would be to deprecate the entire bunch and roll out a replacement that works correctly. Especially for stuff like A_CustomMissile I'd like to rename them anyway and this is a good opportunity.

So let's first collect as many functions that show this problem.

[Major Cooke]

Everything that was in my pull request is a start. All those were affected. The names of the functions are within.

[Matt]

After the reversion the mods I'm trying are still broken - if you are below the monster they'll shoot up, if you're above they'll shoot the ground.

Tested with NBO and nearest starting shotgun guy on map15, which has the following missile state:

Code: Select all

	missile:
		SPOS ABCD 4{
			A_FaceTarget(20,20);
			if(!A_JumpIfTargetInLOS("null")){return state("see");}
			if(A_JumpIfTargetInLOS("null",20)){return state("shoot");}
			return state("");
		}loop
	shoot:
		SPOS E 12 A_FaceTarget(10,10)
		SPOS F 3 light("ZOMBIEATK"){
			A_PlaySound("weapons/shotgf");
			A_CustomMissile("nbobullet",32,0,-1.5,CMF_AIMDIRECTION,pitch+1);
			A_CustomMissile("nbobullet",32,0,0,CMF_AIMDIRECTION,pitch-1);
			A_CustomMissile("nbobullet",32,0,1.5,CMF_AIMDIRECTION,pitch+1);
		}
		SPOS E 6
		goto see

[Major Cooke]

Were they broken before?

[Matt]

Before what? Last version I tried was November 6 and it was fine, but no one's noticed the problem in Hideous Destructor yet (it's a bit harder to see with any of the zombies but it would be instantly obvious with anything that shoots projectiles (and the DERP robot is very obviously broken)) so it seems pretty recent.

I can't think of anything else that might be breaking this than the recent pitch changes.


EDIT: I suppose this was not reverted:

Code: Select all

fixed: A_Face* had the sign for the pitch inverted (In Doom, negative pitch is upward, this had positive pitch upward.)

That behaviour has also been the case for years, so it should be reverted as well if the CustomMissile correction is to be reverted.

[Graf Zahl]

Actually, no. That one got broken with the floatification rewrite. In 2.1.0 and 1.9.0 it was as it is now.

[Graf Zahl]

Were they broken before?

That's the question. I know that during the floating point rewrite some things got broken, and some probably just because all the pitches do not match up and trying to 'fix' it in the wrong place.

I'm going to need some comprehensive test cases that I can run with different ZDoom versions.

[Major Cooke]

Unfortunately the floating point rewrite was far before D4D was even conceived... Otherwise I would've gone back and tried some things.

What I can tell you for sure is, if you recall when you asked me about if A_FaceMovementDirection was working properly, it was indeed 'backwards' at the time. Now the pitch is 'backwards' and faces the wrong direction, hence why in the latest D4D official release (not the zscript one), if you change the bullet tracers to trails, they shoot out going the opposite pitch.

Also the hell knight and baron of hell leaping is broken -- they aim down instead of up now.

That's all I can offer for the moment. I'll have to think of some things.