Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stopwatch functionality to the Timer node #2878

Open
Tracked by #7 ...
likeich opened this issue Jun 18, 2021 · 19 comments · Fixed by goostengine/goost#93
Open
Tracked by #7 ...

Add stopwatch functionality to the Timer node #2878

likeich opened this issue Jun 18, 2021 · 19 comments · Fixed by goostengine/goost#93

Comments

@likeich
Copy link

likeich commented Jun 18, 2021

Describe the project you are working on

An Android launcher. I want to be able to easily keep track of how long the user spends time in each app drawer that my app provides.

Describe the problem or limitation you are having in your project

It requires a stopwatch to keep track of elapsed time, which the Timer node seems like it should provide. It is not intuitive to build a timer that starts whenever a section of the ui is clicked and saving the elapsed time when the app is exited or the ui is clicked again.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Adding a simplistic stopwatch ability to the current Timer node would make it really easy to keep track of time.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I think the best bet would be to add the simplest stopwatch functionality possible and to require the least amount of code. My proposal is to add one new property to the Timer node: time_elapsed. This would complement the already-used time_left property. However, instead of counting down when the timer is started, it would count up.

The time-elapsed property would persist on timer resets from one_shot being false and would only stop counting when the stop() function is called or the timer is started again. The benefit to persisting time_elapsed is that the timeout signal could still be called at specific intervals to update other parts of the code. For example, if the user wants to make a second counter from a label in their game, the wait time could be set to 1 and the label would be updated from the timeout signal.

Speedrun timer example code:
The timeout signal is ignored in this code.

func _ready():
    $Timer.one_shot = false
    $Timer.start() 

func _process():
    $Label.text = $Timer.time_elapsed

Second counter example code:
I'm using the 3.x connection code b/c I don't know how the new connections work.

func _ready():
    $Timer.connect("timeout", self, "update_label")
    $Timer.one_shot = false
    $Timer.start()

func update_label():
    $Label.text = int($Timer.time_elapsed)

Note: The time_elapsed property could be completely ignored and the Timer would still work as it is currently used.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It could, but I think that this is a widely-used capability that would require few enough changes to enable in core.

An example of a stopwatch in code can be found here: https://godotforums.org/discussion/18694/how-to-make-a-stopwatch-in-godot

Another example: https://godotengine.org/qa/3641/how-display-elapsed-time-in-game

Search "stopwatch in godot" for many users asking how to make this functionality in their games.

Is there a reason why this should be core and not an add-on in the asset library?

This solves a very common use case and makes sense being a part of the timer node.

Non-exhaustive list of use-cases:

  • Speedrunning games that display the current run time.
  • Tracking the time taken to beat a level.
  • Tracking the amount of time that a game has been played in total.
@Calinou
Copy link
Member

Calinou commented Jun 18, 2021

Stopwatch functionality requires careful filesystem handling to avoid naming conflicts (or you need to store all timers used by the project into a single file using JSON or similar).

Also, some people may want to prevent timer manipulation while the app is closed, which means encryption would have to be used. As you can see, there's not "one true way" to implement a stopwatch that works across app restarts.

Due to this, I think this should be an add-on instead. Speedrun timers or game time tracking might be fairly common functionality in more polished games, but you generally won't need it in earlier stages of development.

@likeich
Copy link
Author

likeich commented Jun 18, 2021

Stopwatch functionality requires careful filesystem handling to avoid conflicts (or you need to store all times into a single file using JSON or similar).

Also, some people may want to prevent manipulation, which means encryption would have to be used. As you can see, there's not "one true way" to implement a stopwatch that works across app restarts.

Sorry if I was unclear: the time_elapsed property wouldn't persist across app restarts, it would persist across the timer restarting itself after time_left is < zero. When one_shot = false the time_left property is set to the wait_time after the timer times-out to keep going, the time_elapsed property would not be reset when that happens. It would be reset when the timer is started again manually in code.

@YuriSizov
Copy link
Contributor

YuriSizov commented Jun 18, 2021

It is not intuitive to build a timer that starts whenever a section of the ui is clicked and saving the elapsed time when the app is exited or the ui is clicked again.

All you really need to do to track that time is to use OS.get_unix_time() once at the start of the activity, and once again after it has ended, then subtract values.

Using timers would be excessive for long tracked sessions, and if you want to have a real time clock visible, you can just use delta in the process func.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 18, 2021

Speedrun timers or game time tracking might be fairly common functionality in more polished games, but you generally won't need it in earlier stages of development.

This sounds like a good use case to me already. I've also needed this kind of functionality for speedruns.

I foresee that this enhancement won't be implemented in core. Due to this, I think it makes sense to create Stopwatch node and keep Timer implementation clean (even when extending the class via script). Looking at the source of Timer, it's actually not a lot of code:

https://github.com/godotengine/godot/blob/8b692e88726bb7ab114a3a0b1b2ffd973dd4f054/scene/main/timer.cpp

The byproduct of creating a separate implementation for Stopwatch node would be:

  • less hacks to Timer node, which leads to:
  • cleaner and simpler API (autocompletion won't be polluted with unrelated methods such as get_time_left()), which leads to:
  • more readable code, which leads to:
  • no regressions (or regressions minimized, depending on whether the class will have unit tests).

@AaronRecord
Copy link

AaronRecord commented Jun 18, 2021

cleaner and simpler API (autocompletion won't be polluted with unrelated methods such as get_time_left())

@Xrayez I think even if a StopWatch node was implemented, Timer.get_time_left()) should stay.

@YuriSizov
Copy link
Contributor

I think he implies that StopWatch would be unrelated to Timer and therefore will not have this API. In other words, StopWatch will only have stopwatch related methods and properties, and Timer will only have timer related stuff. Instead of polluting Timer with stopwatch related methods as proposed here.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 18, 2021

Yeah, what I mean is that timer/stopwatch functionalities are fairly distinct to warrant making a separate class for both.

If you type "stopwatch" in Google, you'll see both "TIMER" and "STOPWATCH" tabs:

image

So, I'd say these features mutually complement each other. Therefore, this is something Godot could have as well, in theory. But being complete is not part of Godot's development principles, since the development is based on concrete use cases as used by majority of real-life projects using Godot instead, and not because a particular feature complements the other.

This is why I've added this proposal to goostengine/goost#7 as well, because the feature is clearly in alignment with Goost goals.

See also https://en.wikipedia.org/wiki/Unix_philosophy#Do_One_Thing_and_Do_It_Well.

@likeich
Copy link
Author

likeich commented Jun 18, 2021

What I'm proposing is adding only one property to the timer node that keeps track of how long it has been since the timer was last started. That's it.

This would solve my use case of tracking the time and the uses of many users in the forums who's problems I linked to.

I get that time can be tracked by using delta in the process function (that's what the timer node does in the source code), but telling users to deal with that doesn't seem to make much sense to me. Why provide functionality for a timer to count down in a node, but tell users who want to count up that they have to do it themselves?

The current time_left property counts down already, the proposed time_elapsed property would just count up. They're two sides of the same coin.

@AaronRecord
Copy link

AaronRecord commented Jun 18, 2021

The current time_left property counts down already, the proposed time_elapsed property would just count up. They're two sides of the same coin.

I'd rather have a separate Stopwatch node, I think it doesn't really make sense to have a time_left or a wait_time property for a stopwatch. Anyways, you can already kind of get the time_elapsed by doing wait_time - time_left.

@YuriSizov
Copy link
Contributor

@likeich Again, you don't need a timer to know how much time has elapsed. You just need to call OS.get_unix_time() at two points in time and compare values. Using a timer for this is inefficient and wrong. This is what solves your use case.

And for real time counters like that speedrunning example a new node is better be introduced as suggested by others, though it can be done as an addon easily. (Timer node is more versatile than just a controller for counting time, so it makes sense to have in core).

@likeich
Copy link
Author

likeich commented Jun 18, 2021

@pycbouh While not impossible to get around, the problem I deal with doing that is that my code would become much more complex. My launcher works by caching different sections of the ui when they are not in use and removing them from the tree. Using the proposed time_elapsed property would keep me from having to change between these ui sections and saving each time spent whenever a ui section is left. A timer would automatically stop whenever it exited the tree and I would only have to save the time_elapsed once: right before the app closes.

The beauty of the property would be that I could:

  1. Have a timer for each ui section
  2. Start that timer when that scene section is opened
  3. Have any number of ui changes while the time_elapsed would update without any intervention on my part.
  4. Save the last value before the app is closed

I can't imagine a stopwatch requiring a whole new node but I'm interested to hear what others have to say about that

@YuriSizov
Copy link
Contributor

So you rely on the node being removed from the tree to "pause" a timer? I can see why the proposed solution would be complicating things, but that's a bit of a hack itself.

That said, we've probably spent more time discussing it here (combined) than it would take to make a nice and reusable bit for your project. I've made a time tracker plugin for Godot, and it tracks how much time you spend on each main tab (2D, 3D, Script, etc) so I have a point of reference for that sort of task.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 20, 2021

I have implemented Stopwatch node in Goost as previously discussed, see goostengine/goost#93.

image

I can't imagine a stopwatch requiring a whole new node but I'm interested to hear what others have to say about that

If you look at the exact implementation, it's indeed similar to Timer, yet when it comes to concrete cases of measuring time intervals, that's where more complexity is involved which makes it different from Timer. Not all Timer features apply to Stopwatch, especially when we talk about variable names, which are quite important to distinguish for readability. And Stopwatch implementation is twice shorter compared to Timer anyway, so there's not that much binary size bloat as someone might expect.

As a bonus, the editor has separate icons for both Timer and Stopwatch. 🙂

My rule of thumb is: when we have separate objects in the real world that are used differently, this already signifies that those should be implemented as separate classes. Of course, there are types of timers that do include stopwatch functionality, but any universal tool is less useful in solving a specific task effectively, at least that's the way I see it.

@Calinou Calinou changed the title Add stopwatch functionality to timer node Add stopwatch functionality to the Timer node Jun 20, 2021
@YuriSizov
Copy link
Contributor

Btw, Timer node may be removed in a future version, same as Tween has been refactored, see godotengine/godot#41794 (comment)

@KoBeWi
Copy link
Member

KoBeWi commented Jun 21, 2021

Timer can be used without any script, which wasn't true for Tween node. So I don't think it will be removed.

@YuriSizov
Copy link
Contributor

Timer can be used without any script, which wasn't true for Tween node.

Can it though? 🙃 You can set it up and start it in the Inspector, true, but can you actually use it in any meaningful way without a script that reacts to its timeout signal?

@KoBeWi
Copy link
Member

KoBeWi commented Jun 21, 2021

You can e.g. connect timeout to queue_free(). I sometimes create fx nodes like this.

@Xrayez Xrayez closed this as completed Jun 22, 2021
@YuriSizov YuriSizov reopened this Jun 22, 2021
@YuriSizov
Copy link
Contributor

@Xrayez Please don't use magic keywords in your repositories like that as you can actually close the issue here being a member and GitHub doesn't mind 🙃

@Xrayez
Copy link
Contributor

Xrayez commented Jun 22, 2021

Oh sorry, I usually remove closing keywords when I decide to merge PR, my bad.

The only reason why I link PRs like that is because people stumbling upon the proposal can already find a potential solution to the problem, and I've basically resolved it, even if it's not directly solved by Godot core developers officially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants