AlloSphere-Research-Group / allolib

Library for interactive multimedia application development
BSD 3-Clause "New" or "Revised" License
36 stars 14 forks source link

dt units are ms in onAnimate #2

Closed mantaraya36 closed 6 years ago

mantaraya36 commented 6 years ago

Should units be seconds? Also needs documentation.

grrrwaaa commented 6 years ago

Lurker here, but I recommend seconds for consistency with measurements of time elsewhere in the codebase. There is a typedef double al_sec used in a few different locations, why not use that?

https://github.com/AlloSphere-Research-Group/allolib/search?utf8=✓&q=al_sec

Whatever the unit choice, I suggest incorporating that into the argument name. So instead of void onAnimate(double dt) I suggest void onAnimate(al_sec delta_seconds) as being much clearer.

younkhg commented 6 years ago

I also think it should be in seconds. It was my mistake that I didn't fix yet... I'll fix it this weekend.

maybe recommended signature as void onAnimate(double dt_sec)? I'd prefer providing as standard type over alias.

Best, Kee.

On Jan 19, 2018 11:20 AM, "Graham Wakefield" notifications@github.com wrote:

Lurker here, but I recommend seconds for consistency with measurements of time elsewhere in the codebase. There is a typedef double al_sec used in a few different locations, why not use that?

https://github.com/AlloSphere-Research-Group/allolib/search?utf8=✓&q=al_sec

Whatever the unit choice, I suggest incorporating that into the argument name. So instead of void onAnimate(double dt) I suggest void onAnimate(al_sec delta_seconds) as being much clearer.

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/2#issuecomment-359062463, or mute the thread https://github.com/notifications/unsubscribe-auth/AGf3MLeGM1TrpoMEE5yTt6KiOMrkS7kZks5tMOojgaJpZM4Rk4J4 .

younkhg commented 6 years ago

fixed with commit https://github.com/AlloSphere-Research-Group/allolib/commit/c57fb559cd4824eb9a75974a52c58b35544e87b9