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

Execute chunk loading operations outside the main thread #4522

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

szumielxd
Copy link

Overview

Description

Now, main command will wait with further execution until plot center location is asynchronously obtained. This will free the main thread while reaching plot in unloaded chunk

Submitter Checklist

  • Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • Ensure that the pull request title represents the desired changelog entry.
  • New public fields and methods are annotated with @since TODO.
  • I read and followed the contribution guidelines.

@szumielxd szumielxd requested a review from a team as a code owner October 13, 2024 21:00
@github-actions github-actions bot added the Feature This PR proposes a new feature label Oct 13, 2024
Copy link
Member

@PierreSchwang PierreSchwang left a comment

Choose a reason for hiding this comment

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

Can't see any obvious errors, and seems to be working ingame. My only nitpicks currently are

  • Use a Logger instead of Throwable#printStackTrace
  • I think we are using javax Annotations, instead of Jetbrains Annotations (not 100% sure on that though - another review is required either way)

@dordsor21
Copy link
Member

I would address the above things Pierre raised before merge - we should avoid merging code "smells"

@szumielxd
Copy link
Author

szumielxd commented Dec 8, 2024

To be honest, It's hard to get any information about correct code-styling. I can easly find a file with both - NotNull and NonNull annotations. But I'm more than happy to modify my PR to use javax if that's the correct one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature This PR proposes a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants