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

Make Runtime::run Return Lua Values #178

Merged
merged 20 commits into from
Oct 16, 2024

Conversation

CompeyDev
Copy link
Contributor

@CompeyDev CompeyDev commented Apr 11, 2024

This PR intends to return a more meaningful output for Runtime::run invocations. Instead of only returning an ExitCode, Runtime::run now returns a tuple including a Vec of values returned by the lua thread.

TODO:

Once merged, this closes #175 and closes #92.

`Runtime::run` now returns a tuple of both the `ExitCode` (denoting whether the lua thread generally succeeded or not) and also the values returned by it.
This commit removes the chained unwraps introduced previously with a match statement which handles the case when a lua thread may return no values.
@CompeyDev CompeyDev marked this pull request as draft April 11, 2024 13:10
@CompeyDev
Copy link
Contributor Author

CompeyDev commented Apr 11, 2024

@filiptibell In order to return consumable exit information, exit.rs in mlua-luau-scheduler needs to return an ExitStatus instead of an ExitCode.

I'll go ahead and create an issue in the repo.

@CompeyDev
Copy link
Contributor Author

I'm currently returning the raw exit code, instead of an ExitStatus since there doesn't seem to be any way to construct one out of an exit code.

See lune-org/mlua-luau-scheduler#1

@CompeyDev CompeyDev marked this pull request as ready for review April 12, 2024 10:36
I know this is out of the scope of this current PRs, but these minor clippy warnings have been a bit annoying to look at lately, so I've gone ahead and fixed them! :)
@filiptibell
Copy link
Collaborator

Just a heads up - since this is a breaking change it's going to take a while before it gets merged. I've added it to the 0.9 milestone to make sure it's not forgotten for that

@CompeyDev
Copy link
Contributor Author

CompeyDev commented Apr 22, 2024

Would it be possible to merge this before #188?

@filiptibell
Copy link
Collaborator

Would it be possible to merge this before #188?

#188 is not going to have any breaking changes, and the plan is to make a patch release with that PR + changes on main, so it would be difficult to get this PR in before

@CompeyDev
Copy link
Contributor Author

CompeyDev commented Apr 22, 2024

Guess I will restructure once the restructuring is complete.

Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this! Everything looks good to me 👍

@filiptibell filiptibell merged commit df4fb9b into lune-org:main Oct 16, 2024
6 checks passed
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

Successfully merging this pull request may close these issues.

ExitCode is not a useful return type Allow for fetching "context" declarations
2 participants