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

Refactor: Infinite loop in AsyncShaderCompiler according to style guidelines #13319

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

Conversation

hoogmin
Copy link
Contributor

@hoogmin hoogmin commented Jan 31, 2025

A simple refactor of an infinite loop in accordance with Dolphin's style guidelines. As of now, this appears to be the last one in the source. At least according to VSCode's file search functionality.

@Dentomologist
Copy link
Contributor

What if instead we make the loop not-infinite by changing it to while(Core::GetState(Core::System::GetInstance()) != Core::State::Stopping), returning false at the bottom, and replacing the break with return true?

@hoogmin
Copy link
Contributor Author

hoogmin commented Jan 31, 2025

What if instead we make the loop not-infinite by changing it to while(Core::GetState(Core::System::GetInstance()) != Core::State::Stopping), returning false at the bottom, and replacing the break with return true?

I have just made this change. I tested it with Dolphin in its Release configuration. The shader compilation process seemed to work fine. I think it is a good suggestion considering it is the same logic but re-arranged to not be infinite. But if you see any issues, let me know and I will revert them. It's a simple change but I am being cautious to not break anything unintentionally in Dolphin.

Copy link
Member

@jordan-woyak jordan-woyak left a comment

Choose a reason for hiding this comment

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

LGTM. Untested.

@JosJuice
Copy link
Member

JosJuice commented Feb 2, 2025

LGTM if the two commits are squashed into one.

@dreamsyntax
Copy link
Member

@hoogmin bump
You can use your GUI tool or git rebase -i
To squash the two commits into one commit.

@hoogmin hoogmin force-pushed the infinite-style-fix branch from 71816f4 to 7dec083 Compare February 6, 2025 01:08
@hoogmin
Copy link
Contributor Author

hoogmin commented Feb 6, 2025

@hoogmin bump You can use your GUI tool or git rebase -i To squash the two commits into one commit.

I have just done this. Thanks for telling me. I wasn't sure if the git squashing was something I had to do or a maintainer.

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

Successfully merging this pull request may close these issues.

5 participants