search

microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6k stars 2.12k forks source link

Consider using Time.unscaledDeltaTime #79

Closed danglingneuron closed 7 years ago

danglingneuron commented 8 years ago

Just a minor observation: I would recommend considering the use of Time.unscaledDeltaTime in scripts instead of Time.deltaTime. especially input related scripts.

One of the simpler ways for Unity games to pause stuff is to set Time.timeScale = 0. Which stops the calling of FixedUpdate and sets Time.deltaTime = 0. Even when we pause game we still want the HoloLens scripts to work properly specifically input scripts.

One could also put an inspector variable in such scripts so the developer can choose deltaTime or unscaledDeltaTime.

So far for inputs, I only see GazeManager using Time.deltaTime; but I also see UAudioManagerBase and Interpolator script using it as well. Not sure about the Audio stuff, but Interpolator will get effected if I need to pause game and want my pause menu UI to tag along.

NeerajW commented 8 years ago

@danglingneuron good suggestion. Would you like to make the change?

danglingneuron commented 8 years ago

If only I had learnt to use Git and Git tools :( I am still resisting the move from VSTS TFS backend) - will have to learn one day...

NeerajW commented 8 years ago

Have you tried https://desktop.github.com/ - desktop app for GitHub. I really like it. Makes things much simpler.

danglingneuron commented 8 years ago

Will have to learn it, since I have made several changes to HoloToolkit for my project.

jwittner commented 8 years ago

@danglingneuron And if you have any Git questions or issues, feel free to reach out and we can help you through it. =)

stbertou commented 8 years ago

I've used SourceTree (from Atlassian) for years, great tool ! But obviously this desktop tool is a much better option as it will works quite nicely with GitHub's flow

mikthom commented 8 years ago

Something worth considering in the case of Interpolator is that it's so general that it's reasonable to expect it to be used both in cases which should and shouldn't respect time scale.

If you have some object which is moving through your scene at a given rate using interpolator, that should definitely respect time scale, but as @danglingneuron pointed out, there may be cases like pause menus in which Interpolator should ignore time scale.

My recommendation for interpolator would be to add a flag which allows the developer to specify whether they would like scaled or unscaled time. I would expect this to default to scaled time both for backwards compatibility and because that seems to be the more common use case.

A search shows 4 different classes using Time.deltaTime. My recommendations are below:

File Purpose Recommendation
FpsDisplay.cs FPS counter Convert to unscaledDeltaTime
StabilizationPlaneModifer.cs Smoothly moving the stabilization place Convert to unscaledDeltaTime
Interpolator.cs Smoothly translating/rotating/scaling objects Add flag to allow user to specify scaled or unscaled time
UAudioManagerBase.cs Fading audio events Probably use unscaledDeltaTime. Usually messing with audio based on time scales is not the best, but if there are compelling use cases I would be open to hearing about them.
jwittner commented 8 years ago

@micthom-msft or @danglingneuron would either of you like to make this change?

jwittner commented 7 years ago

Closing until someone runs into the issues or someone claims this work.