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

UI: Fix Auto-Config Wizard for custom server with no stream key #10416

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

mar10iana
Copy link
Contributor

@mar10iana mar10iana commented Mar 21, 2024

Description

The issue reported on GitHub involves the OBS Studio Auto-Configuration Wizard becoming non-functional when a custom server is selected without requiring a stream key, preventing further progress. To address this, I included the !custom constraint in the if condition within the UpdateCompleted() function, aiming to handle cases where a custom server is used without a stream key, thereby allowing the wizard to proceed as expected.

Motivation and Context

This change improves the user experience in OBS Studio, especially for those utilizing custom streaming services. It solves the problem outlined in this issue: obsproject/obs-studio#10087, where the setup wizard would inaccurately report readiness due to not considering custom services without a provided stream key.

Fixes #10087

How Has This Been Tested?

This has been tested using a macOS environment.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Please omit the Issue number from the commit message. Please also update the PR title to use the commit message subject.

Please wrap the commit message body at 72 characters, as described in our Commit Guidelines.

In the future, we recommend not submitting patches from your master branch, because submitting from your master branch tends to introduce extra challenges.

Please actually fill out the full PR template. Please also indicate what testing you have done on this code.

@mar10iana mar10iana closed this Mar 21, 2024
@mar10iana mar10iana reopened this Mar 21, 2024
@mar10iana
Copy link
Contributor Author

Thank you! I will make those changes now

@mar10iana mar10iana changed the title UI: Next button enabled, custom server, no stream key (#10087) UI: Next button enabled, custom server, no stream key Mar 21, 2024
@mar10iana
Copy link
Contributor Author

I've updated the PR template and commit message. Please excuse any oversights, as this is my first contribution. If there's anything else that needs adjustment or inclusion, I'll address it as soon as possible. Thank you!

@mar10iana mar10iana requested a review from RytoEX March 21, 2024 17:30
@RytoEX RytoEX self-assigned this Mar 23, 2024
@RytoEX RytoEX added the Bug Fix Non-breaking change which fixes an issue label Mar 23, 2024
@Fenrirthviti Fenrirthviti added the UI/UX Anything to do with changes or additions to UI/UX elements. label Mar 25, 2024
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

This does seem to fix the issue as described.

Commit message nit:

  • Use a verb as the first word after the prefix

I have provided a commit message that addresses that nit and clarifies the reason for the change:

UI: Fix Auto-Config Wizard for custom server with no stream key

The Auto-Configuration Wizard cannot progress when selecting a custom
service that does not require a stream key. Since we have no way to
validate if a custom service requires a stream key, we should just not
require a stream key is custom service is selected. To fix this, check
if a custom service has not been selected when checking if the stream key
is empty.

UI/window-basic-auto-config.cpp Outdated Show resolved Hide resolved
@RytoEX RytoEX added this to the OBS Studio (Next Version) milestone Mar 28, 2024
@mar10iana mar10iana changed the title UI: Next button enabled, custom server, no stream key UI: Fix Auto-Config Wizard for custom server with no stream key Mar 29, 2024
@mar10iana
Copy link
Contributor Author

Thank you! I have update the commit and the title of the PR

@mar10iana mar10iana requested a review from RytoEX March 29, 2024 12:54
@norihiro
Copy link
Contributor

In the commit message, the line
if a custom service has not been selected when checking if the stream key
contains 73 characters. Please fold between stream and key.

The Auto-Configuration Wizard cannot progress when selecting a custom
service that does not require a stream key. Since we have no way to
validate if a custom service requires a stream key, we should just not
require a stream key is custom service is selected. To fix this, check
if a custom service has not been selected when checking if the stream
key is empty.

Co-authored-by: Ryan Foster <ryan@obsproject.com>
@RytoEX
Copy link
Member

RytoEX commented Mar 29, 2024

In the commit message, the line if a custom service has not been selected when checking if the stream key contains 73 characters. Please fold between stream and key.

That appears to be my fault. Probably checked my own work too quickly.

@RytoEX RytoEX merged commit 004253f into obsproject:master Mar 29, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue UI/UX Anything to do with changes or additions to UI/UX elements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-Configuration Wizard with custom Server but no Stream key Next option greyed out
4 participants