OpenTomb icon indicating copy to clipboard operation
OpenTomb copied to clipboard

Timestep-related animation and physics bugs

Open Gh0stBlade opened this issue 9 years ago • 33 comments

I've been experiencing some animation bugs that occur when Lara rolls in either mid-air jump or on floor.

The bug in question results in Lara's rotation being somehow inverted so she's facing the opposite direction she should be. This usually occurs at low framerates.

Gh0stBlade avatar Aug 10 '15 13:08 Gh0stBlade

If a framerate is significantly lower than 60 then animations start to break down. This is because the original engine used fixed framerate and OpenTomb need to simulate that. Using conversions between integer and floating point numbers result in rounding errors and uneven distribution of physical frames between simulated ones. Sometimes simulated frames and/or state changes get lost in process. I did some specific fix to compensate for that in a particular bug, i.e I put the same simulated frame twice in a single physical one. But this isn't a panacea and it still could happen in different circumstances and a very low framerates.

vvs- avatar Aug 10 '15 14:08 vvs-

@vvs- Yes I agree, original games used whole integers. The float issues are also the reason for most of the speed/position bugs even though one was fixed. As @T4Larson also suggested OpenTomb should simulate physics animations at 30 fps, it's the only way these issues due to floating point rounding errors will be solved.

Gh0stBlade avatar Aug 10 '15 15:08 Gh0stBlade

I don't think there is a point in going back to 30 FPS. One of major reasons for developing OpenTomb was to get rid of 30 FPS limit once and for all. Physics and scripts are done with 60 FPS speed, which is not so different from 30 FPS timestep - you usually can divide everything by 2 to make it genuine.

I can only propose "decoupling" of animation engine and actual speed/acceleration calculations, which is halfway done by updating speed/accel only on frame changes. So we now need to move all these Character::freeFalling, Character::moveOnFloor etc. to fixed 60 FPS timestep. And I really mean 60 FPS timestep, cause having three different timebases in one engine is already too much.

Along with speed/animation, Bullet rigid bodies must be also updated on a fixed timestep, i. e. there should be no calls to updateRigidBody and updateGhosts in variable timestep loop. Basically, only thing which should be done on a variable timestep base is updating camera position, animations and graphical effects (animated textures, etc.).

So that's how I see it:

  • Decouple speed and acceleration calculations from variable timestep (already done, but needs to be moved out of frameImpl function somewhere else, out of animation code).
  • Decouple rigid body and ghost object updates from animations.
  • Decouple collision checks from animations.

So the only code which should be executed every frame is animations themselves.

There are also additional things to be done. Currently, I'm not happy how rotate180 effect is managed via "script" "overrides". Override is always a bad thing, if you had it working normally in original. So someone needs to find a way to get rid of these "setAnimCommandTransform" script overrides. However, it's a good function. I think I'll re-use it for tightrope falling animations, cause Core Design guys did their worst and hardcoded Lara position changes for this case; you can tell it by the absence of setPosition animcommands in animations 454 and 457.

Also, we need to solve the issues with momentary changes in animations - like flipping buttons in TR2/3 and turn-180 animation for tightrope. In original engines, animations were played at a fixed timestep with no slerp, so nothing extraordinary happened. But now we can see "rotating" buttons and Lara spinning like hell on her last frame of animation 451. I believe this also must be done on automatical basis - since we have each anim frame specifying rotation values for all the meshes, we must compare rotational values between two frames, and if they exceed 90 degrees, we must turn off slerp for this particular frame switch. It could be costly operation in runtime, so instead I opt for integrating special frame flag which will specify if we should turn off slerp for it. We can also create special function which will force all frames of a particular animation to ignore slerp.

Lwmte avatar Aug 10 '15 17:08 Lwmte

Actually, in the originals, if a Change Position is performed during an animation that has a framerate different of 1, this ugly rotation is visible.

It's not visible if framerate is 1 for obvious reasons (but technically it could be), and it doesn't happen either when the Anim Command is on Frame 0, as there's no interpolation between animations.

Joey79100 avatar Aug 10 '15 17:08 Joey79100

The problem with the current system is not that it operates at a variable rate, but that the simulation resolution can fall below the acceptable level. When it happens, the physical speed could become too slow to catch to a simulation step and that caused it to skip it. And because there are no spare frames expected in the original it performs wrong animations instead.

vvs- avatar Aug 10 '15 17:08 vvs-

@Joey79100: That was expected :) Because framerate value in original structures specify interpolation speed. I. e. framerate of 2 specifies two-times interpolation, framerate of 3 specifies three-times interpolation, etc. Technically it's slerp, but in TRs it was always a fixed-step, while now we have it variable-step.

@vvs-: You are right. This is the second biggest problem of OpenTomb (apart from collisional issues) - it doesn't step the world properly. It was even worse before Nickotte fixed camera shake issues. For lower framerates, there should be a "frame skip" algorithm, or else we will eventually face very ugly issues with physics - like Lara being stuck at floor in infinitely falling position, penetrating the geometry, and so on.

We really need a guy with decent gamedev experience to work on these two issues.

Lwmte avatar Aug 10 '15 17:08 Lwmte

Actually, it just shouldn't skip simulation step at all. Yes, that would cause it to visually slow down but at least it will do the correct thing.

vvs- avatar Aug 10 '15 17:08 vvs-

There already was an attempt to do it right in Engine_Frame:

    if(time > 0.1)
    {
        time = 0.1f;
    }

We need to fix it.

vvs- avatar Aug 10 '15 18:08 vvs-

There already was an attempt to do it right in Engine_Frame:

Because sim substeps are disabled, this clamps the low framerate end to at least one stepSim every 4 game ticks (which means 3 entity ticks pass without physics being done), but may have a couple hundred stepSims called between each entity ticks on high framerates... side effects shouldn't be surprising.

T4Larson avatar Aug 10 '15 18:08 T4Larson

That constant is obviously wrong, that's why I said that we need to fix it. It should be at least 60 Hz.

vvs- avatar Aug 10 '15 19:08 vvs-

I don't fully understand this particular part of the code, or the purpose of it. It's definetly not a frame-skip code, rather it does the opposite job - it "slows down" timescale to 1/10 of second in case we get ridiculously slow speed.

While frame skip should do the opposite - allow the world to "step" with proper amount of time, regarding current execution time. For example, if it takes 1/5 of a second to execute one iteration of main loop, it should "jump" the world exactly at this time interval. It works only for particular parts of the engine - while animations are so-so skipped correctly, physics and collision detection gets completely destroyed on low frame rates.

Lwmte avatar Aug 10 '15 19:08 Lwmte

That's why we can't "skip" simulation steps - it's too complicated. On the other hand, slowing the simulation down allows the engine to catch up properly without complex workarounds.

vvs- avatar Aug 10 '15 19:08 vvs-

It's actually the Bullet's task to properly step its simulation in case of low framerate. We just currently have kind of half-working setup here. For example, test spheres never penetrate geometry and never stuck in weird states in case of low framerate, while Lara can stuck infinitely falling at floor level in such case.

Scripts are also currently done in a "very fixed" timestep, actually they never take actual frame time into account, using scripted FRAME_TIME constant. But this constant must be a variable instead, and updated every frame, because in real world, a frame in which "tick" happens never is fully equal to 1/60 of second.

Lwmte avatar Aug 10 '15 19:08 Lwmte

I didn't look at physics in that context. But the animations won't work if simulated speed falls below 60 Hz. That's exactly the reason for #37.

vvs- avatar Aug 10 '15 19:08 vvs-

Here's a good article which explains all of this.

vvs- avatar Aug 10 '15 20:08 vvs-

It's actually the Bullet's task to properly step its simulation in case of low framerate.

Most entity movements seem to happen outside of bullet's control, often scattered throughout the engine code - and as I mentioned elsewhere, Lara seems to be a static object for bullet - probably no physics transform done by bullet anyway... Right now bullet appears to be only used for collision detection. I also smell a lot of redundancy with the different parts of the code constantly overriding bullet's state with the engine's own entity calculations. I'll have a look at Game_Frame to handle the timing, but as I said elsewhere, this should be done with fixed physics timesteps. The "smooth movement" still happens if things were properly interpolated between actual physics steps (bullet also supports this), but there seems to be a lot other stuff that should be optimized to get this stable, like using a MotionState implementation as a single point of dynamic data exchange between engine and bullet, or scaling the units to bullet's recommended dimensions (see http://www.bulletphysics.org/mediawiki-1.5.8/index.php/Scaling_The_World).

T4Larson avatar Aug 10 '15 20:08 T4Larson

Right now bullet appears to be only used for collision detection. I also smell a lot of redundancy with the different parts of the code constantly overriding bullet's state with the engine's own entity calculations.

Of course, using the (silver) Bullet for everything would be incompatible with Core engines. They didn't use the real physics simulation.

vvs- avatar Aug 10 '15 20:08 vvs-

True. If you trace back OpenTomb development back to 2013, there was Bullet-controlled character controller once. All the speeds were managed in Bullet, all the collision detection, slope detection, and so on. But it was so buggy and flawed (not even talking about being genuine) that eventually TeslaRus completely scrapped idea of Bullet-based character controller, using self-written one instead.

The problem is, original TRs are too ancient to be replicated with modern physics. At least Lara, NPCs and majority of "seems-to-be-dynamic" entities, like boulders. So there are lots of things which should be done manually, including Lara movement.

Eventually, we came to a conclusion that Bullet should be used only for "pure" dynamic entities (like rolling balls, grenades, ropes, etc.), particles, ragdolls, and for collision detection, but not for calculation of actual Lara (or entity) movement. That's the reason why all the rigid bodies are static by default.

Lwmte avatar Aug 10 '15 21:08 Lwmte

Of course, using the (silver) Bullet for everything would be incompatible with Core engines. They didn't use the real physics simulation.

Yet, there's bullet's feedback shoehorned back to the engine (otherwise Lara wouldn't move when shooting at her with mouse1-balls) at multiple occasions (fixPenetrations(), checkNextPenetration() etc., happening here and there...).

That's the reason why all the rigid bodies are static by default.

That would be the reason why they should be kinematic! You have precise control, and you wouldn't confuse bullets actual dynamics with it.

T4Larson avatar Aug 10 '15 21:08 T4Larson

Hmmm... I never got a chance to learn the meaning of CF_... flags. Now I tested it again, and it seems:

  • CF_KINEMATIC_OBJECT just disables body dynamics in case it is set with non-zero mass (i.e. ragdoll). Seems it has no effect on zero-mass bodies whatsoever (I see no difference between static and kinematic - in both cases Lara got pushed by test spheres and slides down the slopes).
  • CF_NO_CONTACT_RESPONSE disables collisions, but seemingly proceeds with callbacks.
  • CF_CHARACTER_OBJECT - don't understand it at the moment.

So far I spotted no difference between CF_KINEMATIC_OBJECT and CF_STATIC_OBJECT, except that it may save us some CPU time, but I'm not sure.

bullet's feedback shoehorned back to the engine (otherwise Lara wouldn't move when shooting at her with mouse1-balls) at multiple occasions (fixPenetrations(), checkNextPenetration() etc., happening here and there...).

We have to ask TeslaRus about collisional approach, cause I have no idea how it's done. There are actual Lara's rigid bodies (green meshes) and also ghost objects (white meshes), and each type of body serves some purpose.

Lwmte avatar Aug 11 '15 00:08 Lwmte

So I assume this is also related to the problem why Lara's position is different every time she pulls up on a ledge? Shouldn't it be the same? Plus Lara is too close to the edge compared to the original that's probably why running forward and jumping is broken?

Gh0stBlade avatar Aug 11 '15 11:08 Gh0stBlade

It can't be the same as long as OpenTomb use non-deterministic time intervals.

vvs- avatar Aug 11 '15 12:08 vvs-

For those who are not familiar with floating point arithmetic here's a small snippet which demonstrates the problem:

#include <stdio.h>

int main()
{
    float x = 0.1;
    if ( x == 1.0/10.0 ) printf("Surprise! You won't see this happen. x = %.27f, %.55f\n", x, 1.0/10.0);
}

I believe that's why Core used fixed point arithmetic in a first place.

vvs- avatar Aug 11 '15 13:08 vvs-

No, I have already described why. PSX is not capable of floating-point operations.

Lwmte avatar Aug 11 '15 13:08 Lwmte

May be that was the real reason, I don't know. But the fixed point still has its' advantages.

vvs- avatar Aug 11 '15 13:08 vvs-

I have recently read somewhere that floating-point numbers are totally unnecessary in games, and usually you can get away with integers. It's just because float calculations are relatively cheap, everybody uses them nowadays.

Animation code is so far only code piece by Core which used fixed-point numbers. But beginning with TR4, they started to use floats more frequently.

Lwmte avatar Aug 11 '15 14:08 Lwmte

So far I spotted no difference between CF_KINEMATIC_OBJECT and CF_STATIC_OBJECT, except that it may save us some CPU time, but I'm not sure.

For bullet, static objects are not expected to change position while kinematic objects are. The difference is that bullet may optimize or simplify dynamic interaction for statics (i.e. they will never "push" other objects around, or be pushed, at least from bullet's point of view). It "may just work" right now, because there's currently little dynamic interaction other than the mouse1-balls, but there's probably more to come - I can imagine you want ropes implemented via bullet - where it reliable dynamics from bullet could matter.

Remember that the physics simulation is often more than just collision and penetration fixes: Dynamic objects are affected by mass, velocities and force vectors to the the simulation right - I usually not the one pointing out that "the Codex must be followed", but whom should I trust if not the bullet developers themselves if they say I should do it the way they designed it :wink:

There are actual Lara's rigid bodies (green meshes) and also ghost objects (white meshes), and each type of body serves some purpose.

Usually, the rigid body parts interact with the simulation (i.e. they may push other rigid bodies around), while the ghost object is the "collision detector" used by the entity/character controller to react to contacts. Currently there seems to be a ghost for every limb, for which collision fixes are applied, this leads to effects where i.e. Lara is moved away from a wall while turning, because a hand strikes a wall and pushes the character away - Lara is not standing right at the wall anymore, and may need to readjust her position for a tricky jump. Often this is not desirable, i.e. in a first-person game, where it would confuse the player when turning, or is too unpredictable for game metrics when i.e. wearing big boxing gloves block your way through doors, and hence a simplyfied shape (capsule) is used.

T4Larson avatar Aug 11 '15 15:08 T4Larson

Ok, I have implemented all the kinds of collision types Bullet offers - including kinematic, actor, static and no contact response. I hope it'll be enough.

As for ghosts... Well, we also went through several "iterations" here. If you'll look at early screenshots of OpenTomb (on SourceForge), you'll see that there was capsule shape used for character collision for some time. However, we had awful bugs with that one. Then we had some other types of ghosts (I don't remember, cause whole collisional code went through at least three rewrites). The problem is, each type of ghost produced its own kind of bugs, and there was never "clear" solution. I think the reason is there was no any fixed ghost sensor in original TRs - instead, they used unique collisional functions for almost every Lara state - it is easily distinguishable if you'll dig into TR1 decompiled code or TR5 debug mappings - there are lots of state functions entitled lara_col_blah, where blah is any state Lara can use.

Lwmte avatar Aug 11 '15 23:08 Lwmte

In Tr4, there is a central structure that accumulates the collisions from all the entities and also from the floor, there's a specific function that deals with the room-geometry and fills the structure with data from 6 sampling points around lara: Floor and ceiling heights, floor tilt etc., imo that's pretty close to a ghost object as you can get with that kind of collision design. The lara_col_* functions handle state-specific actions based only on this collision structure (= ghost object), they're not doing any collision- or geometry checks on their own. I'll check if I have some more information about that...

T4Larson avatar Aug 12 '15 00:08 T4Larson

I have implemented all the kinds of collision types Bullet offers - including kinematic, actor, static and no contact response.

I'm not sure this is correct. By default every object has CF_STATIC_OBJECT flag set. Using logical OR to keep that and add another flag looks weird. Does it make sense for an object to be both static and kinematic?

Here's what Bullet wiki says about it:

Static objects don't move so there is no need to communicate movement. They don't need a motion state. Kinematic objects are controlled by your program and the motion state works in reverse. It communicates movement of your object to bullet so it can detect collisions with it.

vvs- avatar Aug 12 '15 13:08 vvs-