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.

Sign up for GitHub

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

Added pause, resume and exists to control tweens #6

Open
wants to merge 1 commit into
base: pause_tween
Choose a base branch
from
Open

Added pause, resume and exists to control tweens #6

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions tweener/tweener.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ local M = {}
local TYPE_TABLE = "table"
local TYPE_USERDATA = "userdata"

local current_tweens = {}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinking about two solutions, one is a a module state as you did to keep all tweens locally. Other one is to create a table state and return it instead of timer_id now. Like the first one, but I see next cases

The problem what in some scenarios the tween state inside this table will be not deleted

  • game object can be deleted and our cancel will be not invoked
  • user can cancel tween by timer.cancel()
  • Game rebooted in the middle of running tween

For tweener state feels more safety in this cases, wdyt about this?
The cons of state is it requires a table per tween (+memory) and a breaking API for timer.cancel, so need to migrate for tweener.cancel in this cases

The third way also can be a good feature request for Defold and timer.set_pause itself :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was very much thinking outside the scope of Tweener and thinking of my own needs here by using a module state.
I only use tweens to control UI, and it's very limited usage, so a table state would be ideal for my usage. It would also allow usage of string ids for the tweens, so I could not cancel through timer.cancel() only via the module, and to store start, current, end, time elapsed values in this table.
I don't think Tweener needs that (a side project for myself!) and it doesn't solve the issues of game object deletion and game reboots, so a tweener state is safer, and allows the user to check if the timer handle exists.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you have some requirements for the library what you need, probably other users want this too!

I still thinking about the better approach, but in case of return table in tweener.tween instead of module state, I can't ask you to implement the solution in other way :)

Can you explain why you want to use strings as an ID and also for which scenarios do you use the start/current/end time?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's me inventing values off the top of my head! I don't think I have any use of start, current or end time!

I'm currently writing a quiz game, so I'll start a question timer, which I will pause for a little while if the player wants to reveal a hint.

So, in my usage, I would write something like tween.start("timer", start_value, end_value, duration) and then if I want to pause it call tween.pause("timer). I would also tween.get_current_value("timer") even after the tween is stopped/cancelled to get the bonus points achieved. Likely tween.is_finished(id) to control flow routines.

I think all outside the scope of Tweener, but I'm very happy to implement/contribute to Tweener as it's a great learning tool for me. I hope to contribute more to the Defold community in future and give something back for all the help I've received.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you using tweener to track the in-game time? Seems a little bit complex honestly, can a just timer counter solve all this requirements? like

function update(dt)
    if self.is_running then
        self.timer = self.timer + dt
    end
    -- do actions
end

And score mapping will be just tweener.ease() at result/display

In case to share this value with other modules is the special lua module will be more solid than tweener

But anyways the tween pause sounds as a good feature to do! Let's re-make to stateless module (without inner list of tweeners) and return a custom table as a tweener state?

Like this

{
    timer_id = hash[],
    paused = nil,
}

Also the tweener.cancel|set_pause|is_paused will use this new table as argument


---Starts a tweening operation.
---@param easing_function easing_function
Expand All @@ -37,28 +38,39 @@ function M.tween(easing_function, from, to, time, callback, update_delta_time)
end

local time_elapsed = 0

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few of new unintended new lines, better to remove it to keep code style & make changes only for the issue it solving, thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me and the Enter key! Forgot it would create changes!

local latest_time = socket.gettime()

local timer_id = timer.delay(update_delta_time, true, function(_, handle, dt)

if time <= 0 then
timer.cancel(handle)
current_tweens[handle] = nil
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    timer.cancel(handle)
    current_tweens[handle] = nil

We call it twice and we can replace it with already defined function M.cancel(handle)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes complete sense

callback(to, true)
return
end

local current_time = socket.gettime()
time_elapsed = time_elapsed + (current_time - latest_time)

if current_tweens[handle]["paused"] == false then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can check the current state early and make a early return

Currently we do a checks twice for pause state, can be like

        local is_paused = M.TWEENS[timer_id] == false
		if is_paused then
			latest_time = socket.gettime()
			return
		end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully my lua skills improve as we go on :)

time_elapsed = time_elapsed + (current_time - latest_time)
end
latest_time = current_time

if time_elapsed >= time then
timer.cancel(handle)
current_tweens[handle] = nil
callback(easing_function(time, from, to - from, time), true)
return
end

callback(easing_function(time_elapsed, from, to - from, time), false)
if current_tweens[handle]["paused"] == false then
callback(easing_function(time_elapsed, from, to - from, time), false)
end
end)

current_tweens[timer_id] = { ["paused"] = false }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use here a boolean state instead a lua table per easing to reduce memory

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. (sorry I've been commenting on every comment, maybe I should have been clicking Resolve Conversation - first time and thank you for your patience with this complete newbie - your skills far exceed mine and I know you could do this yourself quicker than helping me!)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually "Resolve Conversation" clicked when you push additional changes after a review or resolve the comment in other way. Commenting each comment is also good, no need to excuse

It also kind of fun to make and discuss something together, so don't worry about this also, just have fun with this! You also put your free time into that :)


return timer_id
end

Expand All @@ -85,13 +97,51 @@ function M.ease(easing_function, from, to, time, time_elapsed)
end


---Check if a tween exists and is running.
---@param tween_id hash the tween handle returned by `tween` function
---@return boolean true if the tween is active, false if the tween doesn't exist
function M.exists(tween_id)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think also we can add a function is_paused for the API, like for this is_exists function

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

return current_tweens[tween_id] ~= nil
end

---Cancel a previous running tween.
---@param tween_id hash the tween handle returned by `tween` function
---@return boolean true if the tween was active, false if the tween is already cancelled / complete
function M.cancel(tween_id)
if current_tweens[tween_id] ~= false then
current_tweens[tween_id] = nil
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just current_tweens[tween_id] = nil will be better, no need to check if

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course!

end
return timer.cancel(tween_id)
end

---Pause a running tween.
---@param tween_id hash the tween handle returned by `tween` function
---@return boolean true if the tween was active and could be paused, false if the tween doesn't exist or already paused
function M.pause(tween_id)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pause and resume can be united into one function set_pause(handler, pause)
function can be just one liner

M.TWEENS[easing_id] = not is_paused

If we set a tweens table as a boolean map, <handle_id: is_active>, so we can check in one index about tween state, instead of two and checks for nil

so if M.TWEENS[handle_id]
nil - no running tween
false - tween paused
true - tween active

seems more convinient, wdyt?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entirely makes sense. Like I've done with other code in my project, I was creating the table to store more information - which really would be beyond the scope of Tweener.

if current_tweens[tween_id] == nil then
return false
end
if current_tweens[tween_id]["paused"] == true then
return false
end
current_tweens[tween_id]["paused"] = true
return true
end

---Resume a running tween.
---@param tween_id hash the tween handle returned by `tween` function
---@return boolean true if the tween was active and could be resumed, false if the tween doesn't exist or playing
function M.resume(tween_id)
if current_tweens[tween_id] == nil then
return false
end
if current_tweens[tween_id]["paused"] == false then
return false
end
current_tweens[tween_id]["paused"] = false
return true
end


---@private
---@param easing number[] @The array of easing values, Example: {0, 0.5, 1, 2, 1}. Must have at least two elements.
Expand Down