-
Notifications
You must be signed in to change notification settings - Fork 9
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
builtins: Fix rand
and sleep
functions
#417
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8091c8e
to
7d0be4e
Compare
7d0be4e
to
eab8884
Compare
Fix typo in SVG struct field name: `StorkeDashArray` should be `StrokeDashArray`. The XML field tag used for exporting the content to SVG is correctly spelled.
eab8884
to
5ff9581
Compare
Fix the Evy builtin `rand` function for bad argument (< 1). This must have been a typo or just lapse of concentration because the comment states the correct condition for valid rand arguments.
When calling `sleep` in the JS runtime, enforce a minimum allowed interval of 1 millisecond. This change is motivated by watching students use Evy. I found they experimented with very large and very small numbers. When writing a loop with 10,000,000,000+ iterations containing `sleep 0`, their browser tabs crashed. For example, run this program against `main`: i:num clear "blue" while i < 100000 move i 50 color "yellow" circle 5 i = i + 0.1 sleep 0 end This is enough to freeze the browser tab on my PC for a few seconds, and the Evy UI is unresponsive even after the render finishes. In my branch, it runs smoothly - and slowly - with the STOP button still functional. I experimented with a few values and found a minimum sleep of 1 millisecond (0.001 seconds) is enough to ensure that the browser remains responsive.
5ff9581
to
5a207f8
Compare
camh-
approved these changes
Aug 22, 2024
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 was referenced Aug 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix
rand
andsleep
functions to not crash when used with bad arguments: Inthe case of rand we ensure the argument is
>= 1
, and in the case of sleep weensure we sleep a minimum of 1 millisecond.
Add a minor unrelated typo fix.
This PR has cherry-picked from two other PRs created from forked repo, which
cannot run CI.
Related-pull-request: #412
Related-pull-request: #413
@phy1um - I've manually tested it after the fix with
max
, just like you described inyour PR 🙏 .
Code sample
Docs update:
https://evy-lang-stage-docs--417-lffgt2s0.web.app/builtins.html#sleep