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

Implement recommended thread access to prevent error spam on startup #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isirode
Copy link

@isirode isirode commented Aug 2, 2023

Hi,

The PR as requested.

I removed the thread safety check, since it does not seem necessary anymore.

I tested it on 4.1 official [970459615] (downloaded recently), on a game with multiple scenes, the errors are not present.

Have a good day,

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

This change effectively makes the setup process blocking again, which means project startup is noticeably slowed down (by roughly 1 second every time you start the project).

This needs to be fixed before this PR can be merged, as this add-on shouldn't affect startup times.

@isirode
Copy link
Author

isirode commented Aug 4, 2023

I do not notice it, but I have a pretty good CPU, so it might be why.

Do you know if it is the get_viewport() or the get_viewport_rid() method which is slowing done the startup ?

@isirode
Copy link
Author

isirode commented Aug 4, 2023

Can you indicate me why your fix was not complete also ? It did not produce any warnings on my side when I tested it.

@Calinou
Copy link
Member

Calinou commented Aug 4, 2023

Do you know if it is the get_viewport() or the get_viewport_rid() method which is slowing done the startup ?

The slowdown is mostly in update_information_label(), as getting graphics driver version information is slow.

@isirode
Copy link
Author

isirode commented Aug 5, 2023

Oh, I see, it was previously called inside the thread, but now, it runs inside the main thread.

I did not think of it, since it seemed normal to do graphic operations on the main thread.

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.

Errors in Godot 4.1-stable-mono
2 participants