[2.7.1] Sleepy shotgunguys in "Return to Hadron" WAD

[zalewa]

WAD: http://www.doomworld.com/idgames/levels ... g-i/hadron
Map: E1M9
Skill: 3 (UV)

---

I tested in prboom-plus and they wake up there when they see you. In ZDoom they continue to sleep when you come into view, even when you shoot.

[_mental_]

I found the reason of this issue, even made a pull-request with the fix.
The bug was introduced between versions 1.23 beta 33 and 2.0.13. I guess only Randy knows why it was changes this way...
This particular case was affected by P_PointOnLineSide() only, however P_PointOnDivlineSide() was "fixed" too.
I will really appreciate comments and test cases from anyone who is experienced with the code.

[Graf Zahl]

I do not think that the fix is proper. 'On the line' should be treated as 'in front', but returning 'true' means to shove it to the back, meaning that other maps may now show similar problems.
Just how does it come that these monsters are affected? Which function that calls PointOnLineSide is causing the problem? Is there some potentially bogus reject map at play here?

The underlying issue here is that Doom's PointOnLine functions are horrendously wrong in what they do - ZDoom fixed that - and the only way to get the original behavior back would be to break anything modern.

[Graf Zahl]

Looks like a classic case of depending on an engine bug. Unfortunately this is one that would be really, really costly to reimplement so for now I'll just leave it as it is.
Those shotgunners have barely room to move and with ZDoom's fixed PointOnLineSide functions all directions they may walk into result in a blocked move.

[_mental_]

It may be not the best solution, but could we add a new compatibility flag for it, set for strict compatibility mode only?

[Gez]

Basically, the fix to Doom's logic is that ZDoom uses > instead of >=. Would it really be too costly, performances-wise, to have a compat switch to use >= instead of > depending on a CVAR?

Having a compat option would also allow a real fix for nukemine (instead of disabling some teleporters entirely as is currently the case).

[_mental_]

Yes, this is exactly the same solution I'm thinking of. However, I must say that the number of calls of these two functions can be quite large, like thousands of calls per game tick. It varies greatly from zero to thousands depending on player actions and monster movement.

[_mental_]

As the performance cost of the mentioned compatibility flag addition is appeared to be so negligible, I made a pull-request with it.

Having a compat option would also allow a real fix for nukemine (instead of disabling some teleporters entirely as is currently the case).

Could you please tell me more about the problem?

[Graf Zahl]

Doom had several shortcuts in their PointOnLineSide function for optimization. But these shortcuts were done quite poorly and whether 'on the line' is considered in front or behind does not depend on the line's side but the line's orientation in the map.

ZDoom's function is completely stripped down to the math for the common case. This not only is faster but it also corrects the mistakes in the original code.

But as we see there's always situations where someone inadvertently exploits the quirks of the original one. Other manifestations are certain node bugs in data created by the ancient BSP.EXE.

Now, why isn't this changed? Simple: These functions are quite frequently called and the compat option would add some massive overhead which in 99.9999% of all cases is never needed.

The pull request is not good. Have a look at the original clunker of these functions to understand the problem.

[Gez]

Could you please tell me more about the problem?

See this thread.

[_mental_]

Well, under the "original" algorithm I meant the implementation from old version of ZDoom, before the given issue started to occur.
As for overall approach, I didn't really get it, are you the against the compatibility flag at all or do you want this P_PointOnLineSide()?
As for massive overhead, I would like to know more details what is acceptable and what is not. I agree, that some CPU cycles will be wasted. However I was not aware of any performance restrictions. If there is any, please let me know.

[Graf Zahl]

Yes, if you want compatibility you need 'that' P_PointOnLineSide. Everything else would be incomplete, if not outright wrong.
You actually can find that same code buried somewhere in LinkToWorldForMapthing because for map spawns the problem was so pronounced that it was simply unacceptable. (Hexen has massive amounts of mana items which were lying directly on lines.)

[_mental_]

I added the implementation from the vanilla Doom here.

Apparently, those shotgun guy moved differently in this and in previous versions of my changes. I didn't rebase the PR branch intentionally, so you can compared the behavior quickly.
ZDoom 1.23b33 version is definitely not original but better IMHO. In vanilla, SGs (on UV) block each other sometimes.

[Graf Zahl]

1.23b33 will show similar problems with other maps, though. That change was in response to an old bug report, if I am not mistaken.

[Graf Zahl]

Merged the pull request. We now need a compatibility setting to close this report.