More Pitch Fixes

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.

Post a reply

Smilies
:D :) :( :o :shock: :? 8-) :lol: :x :P :oops: :cry: :evil: :twisted: :roll: :wink: :geek: :ugeek: :!: :?: :idea: :arrow: :| :mrgreen: :3: :wub: >:( :blergh:
View more smilies

BBCode is OFF
Smilies are ON

Topic review
   

Expand view Topic review: More Pitch Fixes

Re: More Pitch Fixes

by Hell Theatre » Sat Dec 24, 2016 3:51 pm

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.

Re: More Pitch Fixes

by Matt » Sat Dec 24, 2016 3:20 pm

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))
?

Re: More Pitch Fixes

by Graf Zahl » Sat Dec 24, 2016 11:56 am

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.

Re: More Pitch Fixes

by Matt » Sat Dec 24, 2016 11:47 am

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.

Re: More Pitch Fixes

by Graf Zahl » Sat Dec 24, 2016 11:22 am

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.

Re: More Pitch Fixes

by Graf Zahl » Sat Dec 24, 2016 8:55 am

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.

Re: More Pitch Fixes

by Rachael » Sat Dec 24, 2016 8:19 am

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)

Re: More Pitch Fixes

by Graf Zahl » Sat Dec 24, 2016 8:12 am

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.

Re: More Pitch Fixes

by Graf Zahl » Sat Dec 24, 2016 7:48 am

So, A_CustomMissile has landed on the chopping block and been replaced by a new function that implements it properly. This one was particularly bad because it would set the projectile's pitch value wrong.

Re: More Pitch Fixes

by Graf Zahl » Sat Dec 17, 2016 4:41 pm

Vaecrius wrote:(Seriously who thought it was a good idea to do it this way anyway???)

We all know... we all know... :?

Re: More Pitch Fixes

by Matt » Sat Dec 17, 2016 4:23 pm

Ugh, thanks but that was opposite of the sort of answer I was looking for. I'll just have to tell everyone to use an earlier dev version of GZDoom until this is resolved. [EDIT: apparently 2.2.0 works with HD if you remove the A_SetInventory and just accept that the thing will constantly crash on exit]
Caligari87 wrote:I think the problem is that it's one of those things where usually the documentation doesn't clearly state "positive up, negative down" or whatever, and so someone using the function for a new project just goes with what works.
For what it's worth, until this happened I've always assumed it was the player that was glitched in having positive values mean look down. (Seriously who thought it was a good idea to do it this way anyway???)

Re: More Pitch Fixes

by Edward-san » Sat Dec 17, 2016 3:45 pm

No, I think ZDoom will add new action functions which would do the correct thing.

Re: More Pitch Fixes

by Matt » Sat Dec 17, 2016 3:34 pm

While a final fix remains pending, can those of us hoping to update our mods assume from hereon in that all pitches will be treated negative up, positive down and that any exceptions to that will eventually be fixed if found?

Re: More Pitch Fixes

by Edward-san » Wed Dec 14, 2016 3:12 am

Graf, do you mind if you test these also with zandronum 3.0 beta (requires commenting out A_FaceMovementDirection)?

Re: More Pitch Fixes

by Matt » Wed Dec 14, 2016 12:35 am

Graf Zahl wrote:First thing I need is some stripped down test stuff I can use to check behavior. If I do not get that I cannot safely determine which one is old and which one is not.
Would something like this do?
Spoiler:

Top