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

Separation of the controls dock from the main window #7278

Merged
merged 8 commits into from
Jun 5, 2024

Conversation

tytan652
Copy link
Collaborator

@tytan652 tytan652 commented Sep 3, 2022

Description

Depends on:

Moves the controls dock UI outside of the main window UI.
And make OBSBasic no longer rely on UI element from the controls dock.

Also adds replay buffer buttons and the pause button in the UI file, those are no longer manually and programmatically created and deleted.

Motivation and Context

This PR has two purpose:

How Has This Been Tested?

I tried to test all the feature of the dock, but please test it yourself too.

Need also some testing about what 2d60c89 was fixing because we no longer rely on UI elements.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Refactor for future features

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.

@tytan652 tytan652 marked this pull request as draft September 3, 2022 08:59
@WizardCM WizardCM added Enhancement Improvement to existing functionality Code Cleanup Non-breaking change which makes code smaller or more readable labels Sep 4, 2022
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from f721b44 to 7fbd452 Compare September 19, 2022 11:57
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from 59b69d7 to 1df1307 Compare October 1, 2022 18:37
@RytoEX RytoEX added this to the OBS Studio 29.0 milestone Oct 1, 2022
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from f5695d3 to 5d5a20e Compare October 18, 2022 06:01
@tytan652
Copy link
Collaborator Author

tytan652 commented Jan 15, 2023

Remove from the milestone, because it is related to the RFC about MVC.
obsproject/rfcs#53

@tytan652 tytan652 removed this from the OBS Studio 29.1 milestone Jan 15, 2023
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from ae45084 to 6d30c97 Compare April 10, 2024 13:40
@tytan652
Copy link
Collaborator Author

The PR has been entirely refactored, it still miss appearance/theming bits for the small buttons of the dock.

@Warchamp7 Warchamp7 self-requested a review April 10, 2024 18:37
@tytan652 tytan652 force-pushed the control_dock_separation branch 4 times, most recently from 8aed488 to 3165559 Compare April 21, 2024 00:07
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from 3bf0dd5 to 992e27a Compare April 21, 2024 09:59
@tytan652
Copy link
Collaborator Author

tytan652 commented May 9, 2024

@RytoEX, done

@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from 9bb85c9 to 2fb32a3 Compare May 11, 2024 23:16
@tytan652
Copy link
Collaborator Author

Renamed noncheckable-buttons.hpp to noncheckable-button.hpp.

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.

I'll give this a test later.

UI/basic-controls.cpp Outdated Show resolved Hide resolved
UI/window-basic-main.hpp Show resolved Hide resolved
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from 1e209d9 to f3fa23b Compare May 20, 2024 20:21
@tytan652 tytan652 force-pushed the control_dock_separation branch 2 times, most recently from 149e758 to df1641f Compare May 26, 2024 00:33
@RytoEX RytoEX self-assigned this Jun 4, 2024
@RytoEX RytoEX force-pushed the control_dock_separation branch 2 times, most recently from 554cc39 to 973c9c3 Compare June 5, 2024 03:17
@RytoEX
Copy link
Member

RytoEX commented Jun 5, 2024

After rebasing, this failed to build due to previous changes in UI/window-basic-main.cpp to void OBSBasic::StartStreaming():

	auto setStreamText = [&](const QString &text) {
		ui->streamButton->setText(text);
		if (sysTrayStream)
			sysTrayStream->setText(text);
	};

The compiler error was:

no member named 'streamButton' in 'Ui::OBSBasic'

This is obvious in hindsight because this PR moves streamButton from OBSBasic to OBSBasicControls. To get this to compile, I removed this line:

ui->streamButton->setText(text);

This does mean that the "Preparing..." text will no longer be shown on the "Start Stream" button. I'm not sure what to do about that.

@tytan652
Copy link
Collaborator Author

tytan652 commented Jun 5, 2024

I added a commit that removes the lamdba, moreover StreamingStarting behavior got split in two by #10633 with a new setText which required moving a signal call and add a new one.

@RytoEX
Copy link
Member

RytoEX commented Jun 5, 2024

I added a commit that removes the lamdba, moreover StreamingStarting behavior got split in two by #10633 with a new setText which required moving a signal call and add a new one.

Thanks for that. Will re-review shortly.

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.

Gave a quick review and local build/run test after the recent force push. Seems fine.

tytan652 and others added 8 commits June 5, 2024 17:39
Avoid using controls dock buttons for recording states. Use signals and
OBSBasic member variables instead.
Avoid using controls dock buttons for streaming state. Use signals and
OBSBasic member variables instead.
Avoid using controls dock buttons as a source for text for system tray
elements.
Avoid tangling controls dock stream button to OBSBasic with a lambda.
Those IDs were used on buttons that were programatically added to the
controls dock, now those buttons are always present inside the UI
file with their own object name.

The use of theme IDs can be replaced by their object names.
@RytoEX RytoEX merged commit 93d5191 into obsproject:master Jun 5, 2024
15 checks passed
@tytan652 tytan652 deleted the control_dock_separation branch June 6, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Cleanup Non-breaking change which makes code smaller or more readable Enhancement Improvement to existing functionality 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.

5 participants