-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add new form of preparation commands that runs before checking for displays #3821
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
base: master
Are you sure you want to change the base?
Add new form of preparation commands that runs before checking for displays #3821
Conversation
|
Thank you for the PR! I think we could just move when the prep commands run instead of introducing another type of prep commands? Edit: You might be interested in helping with this roadmap item LizardByte/roadmap#38 ... if not, this PR can still be accepted in the meantime. |
@FiveYellowMice any response? |
I was travelling last week, hence the silence. Anyways, there are 2 reasons for a new type of prep commands:
Helping out with the roadmap sounds interesting. It seems to encompass the aforementioned "day for extending prep commands". It doesn't seem clear what I can do for the roadmap, though, just by reading its thread. What can I do? |
That's a good point. I was not considering those who are changing resolutions for example. My bad... Expanding on the roadmap entry here. This is just some brainstorming, nothing written in stone. We probably need a few different stages when a command can run.
And the commands and do/undo commands to be refactored a bit. In the apps config it should probably just be a sequence of commands. e.g. "commands": [
{
"detached": True/False,
"elevated": True/False,
"ignore_error": True/False,
"log_file": "e.g. my_command.log",
"name": "Friendly name",
"run": "terminal command to run",
"shell": "shell to use, e.g. auto, bash, cmd, pwsh, etc.",
"stage": "the point when the command should run",
"working_directory": "path to start in"
}
] These would be listed in the order that they should run. In the c++ at each stage loop over the list in order and only run the command if the stage matches the stage we're at. |
Thanks for the additional info. I guess in this case, I can keep the
original plan for this PR, i e. adding a new type of commands, but keep
this future roadmap in mind. Then think about what to do with the roadmap
afterwards.
There does seem to be a backwards incompatibility with the roadmap though.
Currently, the undo prep commands are run in reverse order of definition,
and they will only run if the corresponding do command has exited
successfully. These 2 features allows for graceful exit should one of the
do commands fail. The proposed refactor will discard these 2 features. Has
this been discussed and deemed acceptable?
If not, my opinion is to keep the commands in do/undo pairs, and extend the
pairs to have more type
* Pre-display setup / Post-display revert
* Post-display setup / Pre-display revert
* Pre-stream start / Post-stream stop
* etc.
But that probably makes things more confusing to the user. Some types of
commands (like detached commands) don't fit into a do/undo pair, so they
will not be in a pair, which means more types of data structures, more
complexity.
…On Mon, 5 May 2025, 05:07 ReenigneArcher, ***@***.***> wrote:
*ReenigneArcher* left a comment (LizardByte/Sunshine#3821)
<#3821 (comment)>
If someone have their commands assuming display to be already setup, they
will fail.
That's a good point. I was not considering those who are changing
resolutions for example. My bad...
Expanding on the roadmap entry here. This is just some brainstorming,
nothing written in stone. We probably need a few different stages when a
command can run.
- pre display check - this PR (needed to enable displays before
streaming)
- post display check - current prep commands
- pre stream start - current main command
- post stream start - nothing available currently
- stream pause / disconnect - nothing available currently
- stream quit / end / termination - current undo commands
- additional user connects to stream - nothing available currently
And the commands and do/undo commands to be refactored a bit. In the apps
config it should probably just be a sequence of commands.
e.g.
"commands": [
{
"detached": True/False,
"elevated": True/False,
"ignore_error": True/False,
"log_file": "e.g. my_command.log",
"name": "Friendly name",
"run": "terminal command to run",
"shell": "shell to use, e.g. auto, bash, cmd, pwsh, etc.",
"stage": "the point when the command should run",
"working_directory": "path to start in"
}
]
These would be listed in the order that they should run. In the c++ at
each stage loop over the list in order and only run the command if the
stage matches the stage we're at.
—
Reply to this email directly, view it on GitHub
<#3821 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACFIDUYMADH7LO6C57352V324ZQQZAVCNFSM6AAAAAB37S6OX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNBZGM3TAOJWGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Description
This PR attempts to a new form of preparation commands that runs before checking for displays, to solve feature request https://github.com/orgs/LizardByte/discussions/283 . This will include a minor refactor as suggested in this comment.
Hopefully this can open up doors for adding "on stream pause/resume" commands in the future.
Issues Fixed or Closed
https://github.com/orgs/LizardByte/discussions/283
Type of Change
.github/...
)Checklist