Skip to content

Conversation

@cg2121
Copy link
Contributor

@cg2121 cg2121 commented Dec 27, 2020

Description

This adds a downstream keyer scene type. This is part 1 of
implementing a dsk.

Motivation and Context

Splits #2846 into more manageable separate PRs.

How Has This Been Tested?

Tested with #2846

Types of changes

  • New feature (non-breaking change which adds functionality)

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.

@WizardCM WizardCM added the Enhancement Improvement to existing functionality label Dec 27, 2020
This adds a downstream keyer scene type. This is part 1 of
implementing a dsk.
@jp9000
Copy link
Member

jp9000 commented Dec 29, 2020

Why do you need an explicit new scene type?

@cg2121
Copy link
Contributor Author

cg2121 commented Dec 30, 2020

It makes the code in the UI simpler and doesn't feel as hacky. And it seems to be a more proper way of doing it.

@jp9000
Copy link
Member

jp9000 commented Dec 31, 2020

Forgive me, but could go into more detail on what is/isn't hacky? And why it would be more proper?

DSKs shouldn't be a separate set of scenes. The idea of a DSK is just merely the ability to overlay one scene on top of another. I would envision a DSK feature to just be a list of all of the same scenes that already exist, plus a "None" option at the top or something. Then when you select a DSK scene, it would merely overlay that selected scene on top of the current main scene, that's all I would have imagined it as being.

@mufunyo
Copy link

mufunyo commented Jan 1, 2021

DSKs shouldn't be a separate set of scenes. The idea of a DSK is just merely the ability to overlay one scene on top of another. I would envision a DSK feature to just be a list of all of the same scenes that already exist, plus a "None" option at the top or something. Then when you select a DSK scene, it would merely overlay that selected scene on top of the current main scene, that's all I would have imagined it as being.

Pretty much what I said here:
#1321 (comment)

@cg2121
Copy link
Contributor Author

cg2121 commented Jan 21, 2021

In the dsk PR, there are separate docks for the dsk scenes and the normal scenes, with the idea of having a list of dsk channels. There needed to be a way to distinguish between the two, so that's why I chose to add a new scene type.

It copied the code from the group scene type, so I didn't have to create completely new code and could use it as an example.

The reason for the separate docks, instead of pinning scenes, is so users could easily distinguish between the two. With the single dock, users can't switch between different dsk scenes.

DSKs shouldn't be a separate set of scenes. The idea of a DSK is just merely the ability to overlay one scene on top of another. I would envision a DSK feature to just be a list of all of the same scenes that already exist, plus a "None" option at the top or something. Then when you select a DSK scene, it would merely overlay that selected scene on top of the current main scene, that's all I would have imagined it as being.

This means that there would be two lists for the same scenes, if I'm reading it correctly. What if users have thousands of scenes? Not all scenes would be used as a dsk, so it would take up unnecessary UI space, and what if a user clicks on a scene not intended to be a dsk?

@mufunyo
Copy link

mufunyo commented Jan 21, 2021

What if users have thousands of scenes? Not all scenes would be used as a dsk, so it would take up unnecessary UI space, and what if a user clicks on a scene not intended to be a dsk?

Have you read my linked comment above? It should address all of your concerns here.

@cg2121
Copy link
Contributor Author

cg2121 commented Jan 21, 2021

Yes I did, I just feel having a separate dock is better UI.

@jp9000
Copy link
Member

jp9000 commented Jan 25, 2021

This means that there would be two lists for the same scenes, if I'm reading it correctly. What if users have thousands of scenes? Not all scenes would be used as a dsk, so it would take up unnecessary UI space, and what if a user clicks on a scene not intended to be a dsk?

That's true, although I think that's more of a problem due to the fact that users have no way to categorize scenes. I don't think that should stop one from using scenes. I think if anything, the solution would be to find a better way to categorize scenes in general rather than create a new type of source. You're sort of doing that now via the new source type, but it's still fundamentally a scene, so creating a new source type doesn't necessarily make sense in this context. I think creating a new source type would cause a lot of potential complications down the road.

@Fenrirthviti
Copy link
Member

I've posted some feedback on the RFC itself, as I don't think this is a good way to implement this feature from a UI/UX standpoint. A long discussion was had between dodgepong, muf, dillon, jim, and myself and the outcome of that has been posted on the RFC.

@cg2121
Copy link
Contributor Author

cg2121 commented Jan 26, 2021

Since there is an agreement to not do it this way, I will rework the DSK and resubmit when finished.

@cg2121 cg2121 closed this Jan 26, 2021
@cg2121 cg2121 deleted the libobs-dsk branch November 8, 2021 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants