godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

Add stopwatch functionality to the Timer node #2878

Open likeich opened 3 years ago

likeich commented 3 years ago

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 timeelapsed 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:

Calinou commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

AaronRecord commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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.

YuriSizov commented 3 years ago

Btw, Timer node may be removed in a future version, same as Tween has been refactored, see https://github.com/godotengine/godot/pull/41794#issuecomment-688391554

KoBeWi commented 3 years ago

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

YuriSizov commented 3 years ago

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 commented 3 years ago

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

YuriSizov commented 3 years ago

@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 commented 3 years ago

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.