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

Adds "Expensive Task" API and eager cancelling to client #215

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

Conversation

kralverde
Copy link
Contributor

Description

A Detailed Description:
Adds an idea of "expensive tasks" to the client where the client can watch async tasks that take a lot of compute power so they can be cancelled if no longer needed.

Testing

A description of the tests performed to verify the changes:
Built in debug mode for slow chunk spawning, then flew around.

Checklist

Things need to be done before this Pull Request can be merged.
N/A

  • Code is well-formatted and adheres to project style guidelines: cargo fmt
  • Code does not produce any clippy warnings cargo clippy

@Bryntet
Copy link
Contributor

Bryntet commented Oct 29, 2024

Love this idea! At a cursory glance this implementation looks very good too! Great job :)

@Snowiiii
Copy link
Owner

So actually a Client is just a "tool" for Networking stuff, Kinda like a NetworkingHandler in Java Minecraft. It provides simple functions for sending and receiving Packets. We also use them for all States before Play. I wonder why we should have any Expensive tasks in the Client. If you want to cancel some "Expensive" Player task e.g. Chunk generation, We have already function which get called when the Player closed the connection or we close the Player connection. For Example player::remove() or world::remove_player

@kralverde
Copy link
Contributor Author

kralverde commented Oct 31, 2024

I can port it to player if that's what you really want.

As explained in the discord, player disconnect isn't ideal because it's async and work we don't need will likely be done before it's invoked, it needs to be synchronous.

@Snowiiii
Copy link
Owner

I can port it to player if that's what you really want.

As explained in the discord, player disconnect isn't ideal because it's async and work we don't need will likely be done before it's invoked, it needs to be synchronous.

I would like to have this in the Player, Like i said see the Client more as a util for Networking.

@lukas0008
Copy link
Collaborator

Expensive tasks shouldnt be run inside of async context, it is generally unadvised to do so, this is why rayon exists for example.

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.

4 participants