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.

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

Promise.defer should use task.defer #88

Closed
ghost opened this issue Nov 1, 2022 · 3 comments
Closed

Promise.defer should use task.defer #88

ghost opened this issue Nov 1, 2022 · 3 comments

Comments

@ghost
Copy link

ghost commented Nov 1, 2022

Currently, Promise.defer uses RunService.Heartbeat, which should be replaced with task.defer instead. The current implementation of Promise.defer results in the Promise being resumed in the next frame, rather than the next invocation point.

@evaera
Copy link
Owner

evaera commented Nov 1, 2022

Why?

@ghost
Copy link
Author

ghost commented Nov 2, 2022

Why?

I think task.defer is better suited for what Promise.defer needs to accomplish -- the point is to simply resume a Promise as soon as possible, but not immediately, so resuming in the next frame when you can resume within the next invocation point (now) doesn't seem reasonable.

@Hexcede
Copy link

Hexcede commented Dec 20, 2022

Why?

Here's a few reasons:

  • Resumption happens same-frame (generally just at the end of the frame afaik) which gaurantees ordering across frames
  • Guaranteed to have good task scheduler behaviour (regarding task.cancel)
  • Developer expectations (given the similar naming, you would expect them to both have the same or similar behaviour)

One thing to consider:

  • Deferrals inside of deferrals (e.g. a Promise.defer resulting from a Promise.defer) will happen at the next entry-point but the next entry-point is already the current one
  • The engine has a limit to the number of times you can re-enter a defer point, so the above limits the depth of these calls, thus, you would need special logic to handle that (if you wanted to support defers inside of defers)

P.s. task.defer like all of the other task methods additionally works with threads, so the following is a sufficient replacement for a :Wait():

task.defer(coroutine.running())
coroutine.yield()

@evaera evaera closed this as completed in 9862337 Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants