More Pitch Fixes

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49053
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: More Pitch Fixes

Post by 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.
User avatar
Rachael
Posts: 13523
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her

Re: More Pitch Fixes

Post by 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)
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49053
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: More Pitch Fixes

Post by 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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49053
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: More Pitch Fixes

Post by 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.
User avatar
Matt
Posts: 9696
Joined: Sun Jan 04, 2004 5:37 pm
Preferred Pronouns: They/Them
Operating System Version (Optional): Debian Bullseye
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia

Re: More Pitch Fixes

Post by Matt »

Graf Zahl wrote: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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49053
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: More Pitch Fixes

Post by 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.
User avatar
Matt
Posts: 9696
Joined: Sun Jan 04, 2004 5:37 pm
Preferred Pronouns: They/Them
Operating System Version (Optional): Debian Bullseye
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia

Re: More Pitch Fixes

Post by 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
Posts: 23
Joined: Sat Apr 30, 2016 10:33 am

Re: More Pitch Fixes

Post by 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.

Return to “Closed Bugs [GZDoom]”