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

Update loading in a thread demo for 4.0 #747

Merged
merged 1 commit into from
Jul 10, 2022

Conversation

jtnicholl
Copy link
Contributor

This updates and improves the simple background loading demo (the one that doesn't use load_interactive). On the list in #697.
In addition to updating for 4.0, I made the following additional changes:

  • Make the comments more clear and informative. I removed a few that I thought were just stating the obvious (like one after return tex that just says it returns something, or the one that says ResourceLoader.load loads a resource). Most of them I kept and just edited for clarity.
  • Don't reuse Thread objects. According to the documentation, you shouldn't do this, so the demo now waits for the previous thread to finish (if there is one) and makes a new one.
  • Add an _exit_tree method that makes sure the thread is finished, then frees it.
  • Change the order of the methods in the script to match the order they are called, to make it easier to follow. They are now ordered as _on_load_pressed, _bg_load, _bg_load_done, _exit_tree.

Some of these changes are subjective, so let me know what you think. The last one is technically in violation of the GDScript style guide, since it puts a built-in virtual method after the private methods, but I think it improves readability enough to be worth it. I'm also not sure why the documentation says not to reuse thread objects, since doing so seems to work just fine.

@aaronfranke aaronfranke added this to the 4.0 milestone Jul 10, 2022
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for this! And thanks for improving the engine too :)

@aaronfranke aaronfranke merged commit de27fa1 into godotengine:4.0-dev Jul 10, 2022
@jtnicholl jtnicholl deleted the loading_threads branch July 11, 2022 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants