Skip to content

Add support for global prep commands #977

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Mar 27, 2023
Merged

Add support for global prep commands #977

merged 15 commits into from
Mar 27, 2023

Conversation

pgrunzjr
Copy link
Contributor

@pgrunzjr pgrunzjr commented Feb 25, 2023 •
edited
Loading

Description

Add the ability to add a global set of Prep Commands that will run for all applications. Global prep configuration is accessible in the UI through new tab on Applications page. Users can also enabled/disabled execution of global commands on a per applications basis. Default is Enabled.

Screenshot

Global Command Preparations on Configuration -> General page
image

Per-App Toggle of global prep command execution
image

Issues Fixed or Closed

NA

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

  • I want maintainers to keep my branch updated

@CLAassistant
Copy link

CLAassistant commented Feb 25, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the base branch from master to nightly February 25, 2023 21:33
@github-actions
Copy link

Your PR was set to master, PRs should be sent to nightly.
The base branch of this PR has been automatically changed to nightly.
Please check that there are no merge conflicts

@pgrunzjr pgrunzjr force-pushed the feature/global-do-undo-commands branch 2 times, most recently from cd03c07 to 288fcc9 Compare February 25, 2023 21:37
Nonary
Nonary reviewed Mar 5, 2023
View reviewed changes
Copy link

@Nonary Nonary left a comment

Choose a reason for hiding this comment

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

I have some questions about these changes, I am not an expert at C++ but just curious on the changes as opposed to other common programing structures.

@Nonary

This comment was marked as spam.

Sign in to view
ReenigneArcher
ReenigneArcher requested changes Mar 8, 2023
View reviewed changes
@Nonary

This comment was marked as spam.

Sign in to view
@pgrunzjr
Copy link
Contributor Author

@pgrunzjr The global prep commands tab isn't working with the latest commits. I click on it and nothing happens.

Newer changes have moved global prep commands to the Configuration section and the Apps page tabs removed.

ReenigneArcher
ReenigneArcher requested changes Mar 10, 2023
View reviewed changes
Copy link
Member

@ReenigneArcher ReenigneArcher left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Requesting a few small changes. We're going to need to add boost/json to our build process as well. I will take care of that in another PR so you don't have to dig through all the CI and packaging files.

I like the idea of using that though, can probably move the resolutions and fps options to use it as well, instead of that manual serialize function. I have some tweaks to how the config is handled in the UI in #1023, but I'll merge yours first so you don't have to fix any conflicts.

Edit: Can you also update the documentation, specifically "advanced_usage.rst". This is where we document each setting.

Edit 2: Apparently there's no libboost-json-dev library for debian distros. Do we need to install something additional?

Edit 3: boost-json was introduced in Boost 1.75. This is going to be a problem since Ubuntu 22.04 is on 1.74, and we're building Ubuntu 20.04 with Boost 1.71.

@ReenigneArcher ReenigneArcher mentioned this pull request Mar 10, 2023
Closed
11 tasks
@pgrunzjr
Copy link
Contributor Author

Edit 3: boost-json was introduced in Boost 1.75. This is going to be a problem since Ubuntu 22.04 is on 1.74, and we're building Ubuntu 20.04 with Boost 1.71.

For the purpose of this PR, I think I should be able to update it to use Boost PropertyTree for the JSON stuff for now. In the future, we could look into using another library for JSON, maybe https://github.com/nlohmann/json.

@pgrunzjr
Copy link
Contributor Author

PR updated with documentation updates and to use Boost PropertyTree instead of Boost JSON.

ReenigneArcher
ReenigneArcher reviewed Mar 17, 2023
View reviewed changes
@pgrunzjr pgrunzjr force-pushed the feature/global-do-undo-commands branch from 7658996 to 8d8593c Compare March 19, 2023 14:25
@ReenigneArcher ReenigneArcher force-pushed the feature/global-do-undo-commands branch from 8d8593c to b712970 Compare March 24, 2023 23:43
@ReenigneArcher ReenigneArcher force-pushed the feature/global-do-undo-commands branch from 61dc8eb to 249592a Compare March 27, 2023 13:32
ReenigneArcher
ReenigneArcher reviewed Mar 27, 2023
View reviewed changes
@ReenigneArcher ReenigneArcher merged commit 8c86baf into LizardByte:nightly Mar 27, 2023
@pgrunzjr pgrunzjr deleted the feature/global-do-undo-commands branch March 27, 2023 16:58
@ReenigneArcher ReenigneArcher mentioned this pull request Mar 27, 2023
Merged
11 tasks
@pgrunzjr pgrunzjr mentioned this pull request Mar 27, 2023
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ReenigneArcher ReenigneArcher ReenigneArcher approved these changes

+1 more reviewer

@Nonary Nonary Nonary left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
No open projects
Sunshine-stack
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pgrunzjr @CLAassistant @Nonary @ReenigneArcher