-
Notifications
You must be signed in to change notification settings - Fork 33
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
Adding in Timer #17
base: master
Are you sure you want to change the base?
Adding in Timer #17
Conversation
@@ -0,0 +1,28 @@ | |||
module SDL | |||
module Timer | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just on this line extend self
instead of writing self.
at every single method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer explicit self.
definitions over extend self
, which only has value if the module is meant to be used as an includable module and a namespace for global methods at the same time —it may be valid here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto ^ 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we have monotonic clocks and evented loops, what do SDL timers provide specifically?
Furthermore, do callbacks run from a Crystal context? I didn't check the documentation.
I'm not sure since I don't know what a monotonic clock is, but if SDL provides it, then the API should as well. I don't think the callbacks are in a crystal context since they're defined in C. But I didn't need to use any callbacks in the samples. |
Well, SDL provides a network library, which is nice for C (platform abstraction) but they'll mess up with the Crystal event loop. We shouldn't integrate every SDL library, especially when Crystal already has an alternative. The following will probably behave better (crystal context, integrated into the event loop), for example: def interval(seconds : Time::Span)
spawn do
sleep(seconds)
yield
end
end
interval(5.milliseconds) do
# ...
end Instead of SDL ticks, there is |
end | ||
|
||
def self.delay(ms : UInt32) | ||
LibSDL.delay(ms) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDL_Delay()
duplicates sleep()
and AFAIK block the current thread, thus block the crystal event loop.
module Timer | ||
|
||
def self.add(interval, &callback : UInt32 -> Void) | ||
LibSDL.add_timer(interval, callback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid callback
will be run from an unknown context, possibly another thread (unknown to crystal, thus unknown to the garbage collector, and incapable to access the event loop).
It also relies on a captured block passed to a C function, which will make it impossible to rely on closure variables, and there may be GC issues (e.g. callback trying to access collected variables) making it complex to handle, over a simple crystal function, such as spawn { sleep(seconds), yield }
for example.
start_prompt_surface = font.render_shaded("Press S to Start or Stop the Timer", font_color, bg_color) | ||
pause_prompt_surface = font.render_shaded("Press P to Pause or Unpause the Timer", font_color, bg_color) | ||
|
||
class SampleTimer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sample timer is very nice, but can be achieved using Time.monotonic
and doesn't need SDL_GetTicks
. For example:
ticks = Time.monotonic.total_milliseconds
Ah, I see! Ok. That makes a lot of sense. If that's the case, then maybe we can just have the sample timer run the same way, but with a crystal context, and then just ditch the whole SDL Timer addition? If you're cool with that, I will make that change. Keeping the samples with the same result, but using straight Crystal, and remove the timer. |
This adds in the SDL_Timer stuff. Used for checking elapsed time in milliseconds, and creating delayed callbacks. I added 2 samples for doing a simple timer, and a more advanced timer from the Lazy Foo tutorial 22 & 23.