More Pitch Fixes

[Graf Zahl]

For this one I need a quick review:

Checking A_FireBullets I noticed that AimBulletMissile used the negative of the pitch to calculate the trajectory. But this pitch came from A_Face which was definitely wrong and got fixed a few weeks ago. So I'd appreciate if someone who had used the projectile feature of A_FireBullets to check if the current code is ok.

[Rachael]

It is not. Which is why I made a pull request reversing it.

I just tested that guy's mod again today with the latest QZDoom build and it is still reversed.

(EDIT: You seem to have put in the same fix I did - which, when I tested it, it was correct)

[Graf Zahl]

Thanks for confirming. Just as I suspected. When this code was added, A_Face was wrong so this negation was a counter-fix for the broken A_Face.

[Graf Zahl]

Ok, then. I just finished reviewing the pitch mess.
Fortunately the only functions that had a broken interface were A_CustomMissile and A_FireCustomMissile, but there were several places where some bad compensation was hacked in to work around those bugs and the faulty model positioning.

MC's PR identified almost all of them correctly, but I fixed the one in P_RailAttack differently so that the bogus stuff does not propagate throughout the function.

And I just have to reiterate: What was Randi thinking when doing the pitch this way? Looking through the code it always requires negation to get it right. The natural pitch would have been positive upward.

[Matt]

The thing is, now we are in the worst possible situation: There's a lot of content out there that was made for the old and correct versions and some new content for the broken 2.2

What's the old content that got this right? I just dug up a version of HD from September 29, 2014 and the latest ZDoom dev build breaks it in the same way (go to map12, summon a "DrunkImp" on top of the silver platform and stand below it and watch it shoot its projectiles up to the sky instead of down to you) - as far as I've been aware it had been like this for years.

[Graf Zahl]

That's why I replaced A_CustomMissile with a renamed version that works correctly and deprecated the old one. Broken is broken, no matter how you abused the bug.

[Matt]

So does this mean those of us whose mods this fix breaks can go ahead and treat all instances everything other than A_[Fire]CustomMissile as positive pitch meaning pointing down, and we should replace for instance

Code: Select all

A_FaceTarget(0,0)
A_SpawnItemEx("DoomImpBall",0,0,32,cos(pitch),0,sin(pitch))

with

Code: Select all

A_FaceTarget(0,0)
A_SpawnItemEx("DoomImpBall",0,0,32,cos(-pitch),0,sin(-pitch))

?

[Hell Theatre]

First and foremost you should test your code. There were several places where pitch got inverted and it's hard to say how these interact with each other.

If I understand it correctly, it all originated by some clueless code inherited from Skulltag for models, and nobody realizing that it got its pitch messed up and 'adjusting' other code for it, breaking it in the process. Needless to say, once the original bug was fixed the whole thing started to unravel because all the inconsistencies that got introduced created an utter mess of contradicting code.

There's really just two choices here. Treat compatibility as a holy cow and leave the mess as it is, effectively preventing any sane use of it or fix it for real and ask the few projects that got affected to get fixed.
There's absolutely no use trying to mess this up any further, I think what Graf did with the two A_CustomMissile functions was the best compromise - and the new ones even have nicer names.