PS: please consider using Arcanist to submit patches. By using that, we'll be able to see the context of your patch (i.e. where it now says "Context not available", you'll be able to click & see more code). It helps us reviewing.

The patch is getting there, just a few more tidbits to clean up.

In D728#32569, @Sybren A. Stüvel (sybren) wrote:

I think we can still clarify some things.

• "clock time" is vague, as all time is measured by some clock. I suggest changing it to "game time".

I'm not responsible for the API name and the current documentation.

• "render time" usually does not refer to a point in time, but rather refers to a duration: the time it takes Blender to render a frame. Let's not use that term here, and just use "the current game time" instead.

It is not the current game time. It is really the game time corresponding to the next frame rendered. The current game time, from the logic point of view is "frame time".

• I still don't understand what "frame time" is, and in what way it is different from "clock time". The docstring refers to the time of the last physics/logic tick, but since there can be multiple physics ticks per logic tick, this is also ambiguous, and conflicting with the comment "Returns current logic frame clock time" in KX_KetsjiEngine.h. Apaart from this, both the comments in the CPP code as well as the docstrings for the Python wrapper should explain whether it's game time or wall clock time that is referred to here.

The "documentation" said you can have multiple physics ticks per logic tick, but it is not what I understand from the code. In next frame, I see sequential call to logic than physics call, without any conditional. But I may miss something. I can keep only "logic tick".

• The set/getTimeScale functions don't document *which* time is multiplied. This should mention "game time".

It modifies game time in general (i.e. clocktime and frametime). And I don't see what it can modifies otherwise, as the only other time is "real" time, which won't be real if we can modify it.

In D728#32574, @Arnaud Degroote (adegroot) wrote:

I'm not responsible for the API name and the current documentation.

My idea was that, since you are actively working on this, and wrapped your mind around which thing means what, you're the perfect person to clarify things. And I feel (but this is a personal thing) that the person making things more complex is also responsible for making things easier to understand.

It is not the current game time. It is really the game time corresponding to the next frame rendered.

I don't understand. How is this possible, when the time the next time will render at is depending on the logic code that's currently running?

The "documentation" said you can have multiple physics ticks per logic tick, but it is not what I understand from the code. In next frame, I see sequential call to logic than physics call, without any conditional. But I may miss something. I can keep only "logic tick".

Bullet can actually perform multiple physics steps per call, I think it's hidden in there.

It modifies game time in general (i.e. clocktime and frametime). And I don't see what it can modifies otherwise, as the only other time is "real" time, which won't be real if we can modify it.

That's pleasantly consistent. So now you and I know this, but I feel that this still should be documented; the BGE doesn't always make sense, so even when it does it's nice if it's explicitly documented. The docstrings have my preference, but you could also just update the RST files.

@Arnaud Degroote (adegroot) I've opened up a discussion on time handling in general in the BGE; the more I look at it, the more I see a need to clarify things. Maybe you want to join in the discussion as well? Please see the comments at D1509.

In D728#33085, @Sybren A. Stüvel (sybren) wrote:

Those are some nice additions to the comments. It clarifies a lot, and also clarifies where things are unclear :)
Thanks!

What is the next step for this patch so ?

As far as I'm concerned, we should just accept this patch and merge it with the master branch. Bigger time management overhauls for the BGE will come later. The next step is for @Jorge Bernal (lordloki) and/or @Porteries Tristan (panzergame) to either accept this patch, or explain why they don't.

@Arnaud Degroote (adegroot): Can you add these functions in the python rst doc file for bge.logic module ?

See :

I found a bug when you accel at start, slow and accel again, the second accel is bigger than the first, i checked with logic.getTimeScale : it's the same value.

In D728#35849, @Porteries Tristan (panzergame) wrote:

See :

test_time_speed_bug.blend471 KBDownload

Press up arrow to accel, down arrow to slow and 0 to reset time scale.

I found a bug when you accel at start, slow and accel again, the second accel is bigger than the first, i checked with logic.getTimeScale : it's the same value.

It should be fixed now. Thanks for the report.

In D728#35872, @Arnaud Degroote (adegroot) wrote:

• [doc] Document the new API about time in bge.logic.rst

Done in f96b898e32ce

I made some code style remarks.

Added some notes. The logic seems sound, so now it's just a matter of making a good, clean API.

Any hope to see it merged before 2016 ?

I know that it is a volunteer project, and that everyone has a lot of other stuff to do. But, on other side, my initial mail on the mailing-list (with the associated patch) was in November 2013 (more than 2 years ago), and the logic change is about 5 lines of code (and it is pretty clear that, in the default setting, it does change nothing). It is hard to keep motivated to contribute to BGE in these conditions.

Anyway, best regards,