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

Add API to cancel async jobs #14602

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Add API to cancel async jobs #14602

wants to merge 24 commits into from

Conversation

y5nw
Copy link
Contributor

@y5nw y5nw commented Apr 30, 2024

This PR adds an API to cancel async jobs.

To do

This PR is a Work in Progress.

  • Implement replacing jobs (Dropped)
  • Implement job cancellation
  • Add tests for
    • Replacing jobs before the job is run (Dropped)
    • Replacing jobs after the job is complete (Dropped)
    • Canceling jobs
  • Add documentation

How to test

The new API should be covered by unittests.

Note that I have not yet figured out how to test certain things mentioned in the PR, such as canceling existing async jobs on shutdown.

@sfan5 sfan5 added Feature ✨ PRs that add or enhance a feature @ Script API labels May 2, 2024
@Zughy Zughy added the Concept approved Approved by a core dev: PRs welcomed! label May 2, 2024
@y5nw y5nw marked this pull request as ready for review June 3, 2024 08:38
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

some thoughts

doc/lua_api.md Outdated Show resolved Hide resolved
doc/lua_api.md Outdated Show resolved Hide resolved
src/script/cpp_api/s_async.h Outdated Show resolved Hide resolved
src/script/lua_api/l_async.cpp Outdated Show resolved Hide resolved
@y5nw
Copy link
Contributor Author

y5nw commented Jun 3, 2024

I almost forgot: should this feature also be available for the main menu environment?

@sfan5
Copy link
Member

sfan5 commented Jun 3, 2024

I almost forgot: should this feature also be available for the main menu environment?

No need atm. If someone needs it in the future, they can add it.

doc/lua_api.md Outdated Show resolved Hide resolved
@y5nw y5nw changed the title Add API to replace an existing async job Add API to cancel async jobs Jun 3, 2024
games/devtest/mods/unittests/async_env.lua Show resolved Hide resolved
src/script/lua_api/l_async.cpp Show resolved Hide resolved
src/script/lua_api/l_async.cpp Outdated Show resolved Hide resolved
src/script/lua_api/l_async.h Outdated Show resolved Hide resolved
src/script/scripting_server.h Outdated Show resolved Hide resolved
@sfan5 sfan5 self-assigned this Jun 4, 2024
y5nw and others added 3 commits June 4, 2024 14:25
Co-authored-by: sfan5 <sfan5@live.de>
doc/lua_api.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature @ Script API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants