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

Jump to bottom

Update spr.lua #1

Closed
wants to merge 1 commit into from
Closed

Update spr.lua #1

wants to merge 1 commit into from

Conversation

howmanysmall
Copy link

  • Prefer against local variables in Luau.
  • Don't need to recalculate 2 * pi every time.
  • Prefer string.format to ():format.
  • Prefer table.create for toIntermediate functions.

@howmanysmall
Update spr.lua
d656d1a 
- Prefer against local variables in Luau.
- Don't need to recalculate `2 * pi` every time.
- Prefer `string.format` to `():format`.
- Prefer `table.create` for `toIntermediate` functions.
@Fraktality
Copy link
Owner

Regarding math: This library can be used in environments where getfenv and setfenv may be called, which defeat the builtin optimization.

Regarding tau, we may do better pulling (2*pi) out to the front of the expression to let Luau perform constant folding.

Is the purpose of the string.format change speed? These are error messages, so the shorthand syntax seems reasonable. I don't have strong opinion.

Regarding toIntermediate, this is a bizzare way to fill an array when you know the elements ahead of time. I'm not sure what the intent was.

@Fraktality Fraktality closed this Aug 10, 2020
@Fraktality Fraktality added the wontfix This will not be worked on label Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
wontfix This will not be worked on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@howmanysmall @Fraktality