search

opentomb / OpenTomb

An open-source Tomb Raider 1-5 engine remake
http://opentomb.github.io/
GNU Lesser General Public License v3.0
1.39k stars 145 forks source link

Slide direction is messed up #174

Closed Lwmte closed 8 years ago

Lwmte commented 9 years ago

When Lara slides, her orientation chaotically changes, but mostly she end up in opposite direction to slide direction.

vvs- commented 9 years ago

It's not fixed. Try to slide in Colosseum for example.

vvs- commented 9 years ago

The sliding in The Great Wall doesn't work at all anymore.

vvs- commented 9 years ago

Also, it doesn't work in Jungle. I see two blended Lara models looking in opposite directions trying to slide simultaneously. Related to #178.

vvs- commented 9 years ago

No, actually it doesn't fix neither.

stohrendorf commented 9 years ago

Yep, famous last words, as I said :(

vvs- commented 9 years ago

It's a collision bug related to #182. Lara is pushed back and orientation is changed like it should when she stumbled on an impassable slope.

vvs- commented 9 years ago

May be the angle of a slope is messed up again somewhere.

vvs- commented 9 years ago

Also, try to step on a slope. It works incorrectly.

vvs- commented 9 years ago

Also, when Lara jumps using slopes her orientation now changed randomly because of this bug.

vvs- commented 9 years ago

Removing ghost objects partially fixed this too. Lara can slide now but orientation is still wrong.

vvs- commented 9 years ago

When Lara steps on a slope she now always turns 180 degrees. It doesn't matter what orientation she had before.

Lwmte commented 9 years ago

Now it's better, but one more thing that is incorrect.

Try to launch WALL.TR2. Lara should slide three times and in the end face the cave opening. But she does last slide wrong. This bug wasn't present before refactoring.

vvs- commented 9 years ago

Bullet 2.82 has horrible bugs, e.g.:

#define SIMD_PI           btScalar(3.1415926535897932384626433832795029)
#define SIMD_2_PI         btScalar(2.0) * SIMD_PI
#define SIMD_HALF_PI      (SIMD_PI * btScalar(0.5))
#define SIMD_RADS_PER_DEG (SIMD_2_PI / btScalar(360.0))
#define SIMD_DEGS_PER_RAD  (btScalar(360.0) / SIMD_2_PI)

That means that conversion from radians to degrees is horribly broken.

vvs- commented 9 years ago

It's fixed in Bullet 2.83. That's a direct result of switching from in-tree Bullet to system library.

stohrendorf commented 9 years ago

:frowning:

vvs- commented 9 years ago

It seems fixed now. Could it be closed?

Lwmte commented 9 years ago

But I have Bullet 2.83, and this bug still exists.

vvs- commented 9 years ago

I can't reproduce it on any level. Everything works just as it should.

vvs- commented 9 years ago

I'm using Bullet bulletphysics/bullet3@1fa4819afc806cd8a7081cd48943fc9d0c25bb92

stohrendorf commented 9 years ago

@vvs- seems to have a bug free version running! Joke aside, I think somewhere is a tiny memory corruption that rarely manifests. But it could also be fixed with 89a3b63fcb00b1523044e359456d8dc281fc6bb2.

vvs- commented 9 years ago

Or it could be a problem with optimization. We already had such issues before.

stohrendorf commented 9 years ago

BTW, that's why I use constexpr and enum class wherever possible in my code (not in OpenTomb yet), and I think it would be good to #undef OpenTomb at some time.

vvs- commented 9 years ago

Please, reopen this if you still have this issue.

Lwmte commented 9 years ago

Yes, it still exists. Do you need a video demonstrating the bug?

vvs- commented 9 years ago

Yes, it would be helpful.

vvs- commented 9 years ago

After testing the engine with different compiler optimization levels I came to a conclusion that anything higher than -O0 produced buggy code.

stohrendorf commented 9 years ago

Running OpenTomb with valgrind in RelWithDebInfo (which is -O2) showed some interesting behaviour:

  1. When jumping up, the first animation is played twice.
  2. When landing, the animation stops, but Lara keeps on sliding, until this happens (without Lara being killed): slide-anim-bug

But valgrind didn't show any problems regarding sliding, except that in BtEngineClosestRayResultCallback::addSingleResult, the first if seems to depend on uninitalized values, which could be the reason for many bugs.

vvs- commented 9 years ago

Exactly. All optimization bugs seems to be related to collisions.

stohrendorf commented 9 years ago

Seems like Res_GenRoomCollision is broken.

Lwmte commented 9 years ago

Collision bugs is a well-known issue, it was tested by TeslaRus and he said that all optimization levels above -O2 produce completely unworkable code (funny thing that he recently said to me he was able to produce stable build with MinGW-64 and -O3).

Seems that any optimization in Bullet is generating buggy code. But we really don't know yet if it's Bullet or OpenTomb itself which is buggy.

Lwmte commented 9 years ago

Here is what happens: http://youtu.be/d33oCh2hFIM

Actually, when I recorded this video, something even more weird happened, as Lara stucked at sector border, and moreover, game crashed when I entered noclip mode (it happened at the end of the video, when Fraps obviously stopped capturing, since app crashed).

Checked it again - yes, Lara gets stuck at this point every time I record the video, without recording she simply faces opposite last slope.

And for everyone to be sure - yes, this didn't happen before refactoring.

stohrendorf commented 9 years ago

Did it crash with 322110f4af8c8c6488436839378f7b6cec3c0fc6 or later? Because I have added a simple destruction check in EngineContainer there that throws an uncatched std::runtime_error if it detects that the container was destroyed (which isn't necessarily always detected due to the nature of bools).

vvs- commented 9 years ago

Some collision bugs were fixed recently. Is this issue still reproducible?

Lwmte commented 9 years ago

Yes, I still get the same issue.

Lwmte commented 9 years ago

I still get a bug, even with margins.

Also I finally was able to capture the bug on video! Look here: http://youtu.be/hMP9rXnbot4

It proves that it doesn't depend on 60 FPS lock, but indeed with active recording (i.e. 60 FPS lock) it occurs much more rarely, sometimes it works as it should in such case, but sometimes it sets even more weird collision angles, like 90 degrees off the right one, etc.

stltomb commented 9 years ago

Can you try it again with 840e83331bc4c3c0574ae8f78f2dc0846bda8476 and 73f9970660dbf5f4e3822637e3b3a072638f1798 applied (the latest master)? I can't reproduce this now.

EDIT: Actually, I couldn't really reliably reproduce the change of direction, she always got stuck on the last slope.

Lwmte commented 9 years ago

No, unfortunately, bug still exists... I did a clean release build, and still the same. By the way, sorry about that error with ghost objects. I was a bit lost among new code back then! :smile:

stltomb commented 9 years ago

Can't get it to reproduce, I tried compiling it with gcc and clang, with Debug and RelWithDebInfo and still nothing. However, if I let her slide backward, she starts facing forward on the next slope (TR2 Wall). Is this how it should work? The same thing happens on the build from before the refactoring.

stohrendorf commented 9 years ago

From looking at the code, I'd say that when sliding down backwards, the animation for sliding down with Lara's back first is rotated by 180 degrees (see character_controller.cpp:995). What needs to be done is to rotate her again by 180 degrees when she stops sliding.

Gh0stBlade commented 9 years ago

If you roll onto a slope, it also messes up the slide direction.

Lwmte commented 9 years ago

Currently, almost everything messes up slide direction and/or animation, because sliding checks and calculations are done outside state control module. It means that Lara animates slide and actually slides in two different parts of the code.

It should be fixed by adding slide animation switches to the proper state functions. However, it won't fix slide direction.