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

Document known bugs with the Multi-Threaded thread model project setting #75365

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Mar 26, 2023

  • Add a warning on project startup when using the Multi-Threaded option.
  • Improve command line validation for the --render-thread CLI argument.

@Calinou Calinou requested a review from a team as a code owner March 26, 2023 17:42
@Calinou Calinou added this to the 4.1 milestone Mar 26, 2023
@Calinou Calinou force-pushed the doc-thread-model-multi-threaded-bugs branch from 3f8278a to 67a0052 Compare March 26, 2023 17:45
@Calinou Calinou requested a review from a team as a code owner March 26, 2023 17:45
@Calinou Calinou force-pushed the doc-thread-model-multi-threaded-bugs branch from 67a0052 to 721b029 Compare March 26, 2023 17:57
@YuriSizov
Copy link
Contributor

I would like someone from @godotengine/core to sign off on the main.cpp changes.

@RandomShaper
Copy link
Member

I'm wondering if it wouldn't be better that, in case separate thread is requested, the engine forces single-safe and prints a warning telling about the "downgrade."

@Calinou
Copy link
Member Author

Calinou commented May 6, 2023

I'm wondering if it wouldn't be better that, in case separate thread is requested, the engine forces single-safe and prints a warning telling about the "downgrade."

On the other hand, we may want to fix this option to be eventually usable in the long term 🙂

So that's why I wouldn't prevent using it entirely.

@RandomShaper
Copy link
Member

What I mean is, multi-threaded is kept as a pickable option, but until it's fixed, the engine assumes the closest safe option and informs the user about the matter.

@Riteo
Copy link
Contributor

Riteo commented May 9, 2023

@RandomShaper tbf I'm pretty divided on this one: on one hand it would be the most consistent and useful choice but I'm seriously worried that once the thing actually becomes useful that people with it on (but what previously was due to this actually single thread mode) would eventually get hard-to-debug issues with it.

main/main.cpp Outdated Show resolved Hide resolved
@Calinou Calinou force-pushed the doc-thread-model-multi-threaded-bugs branch from 721b029 to 21c4a0f Compare June 13, 2023 16:06
main/main.cpp Outdated Show resolved Hide resolved
- Add a warning on project startup when using the Multi-Threaded option.
- Improve command line validation for the `--render-thread` CLI argument.
@Calinou Calinou force-pushed the doc-thread-model-multi-threaded-bugs branch from 21c4a0f to 0613978 Compare June 14, 2023 06:37
@akien-mga akien-mga merged commit 0ebd2ae into godotengine:master Jun 14, 2023
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants