Skip to content

frontend: Log sceneitem and filter enabled#11135

Open
Penwy wants to merge 2 commits into
obsproject:masterfrom
Penwy:log-item-enabled
Open

frontend: Log sceneitem and filter enabled#11135
Penwy wants to merge 2 commits into
obsproject:masterfrom
Penwy:log-item-enabled

Conversation

@Penwy
Copy link
Copy Markdown
Contributor

@Penwy Penwy commented Aug 14, 2024

Description

This logs whether sceneitems and filters are enabled on scene collection load.
Inactive ones are prefaced with a o and active ones by a *.
An example log can be found here

Motivation and Context

Whether or not sceneitems/filters are enabled is often an important matter in support, having it logged would greatly simplify some support workflows.

How Has This Been Tested?

Running obs, checking it logs correctly.

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.

@prgmitchell
Copy link
Copy Markdown
Member

This is just my opinion of course but I think if this PR is introducing state logging on load it should also include logging of state changes.

@alinsavix
Copy link
Copy Markdown
Contributor

Would it perhaps be better to identify enabled or disabled filters just by adding (disabled) or (enabled) somewhere on the line? Or even just (disabled) for the disabled ones, with lack of it implying not-disabled?

That way no special knowledge is needed to read that information in the log, the filter lines are just self-explanatory.

@Penwy
Copy link
Copy Markdown
Contributor Author

Penwy commented Aug 14, 2024

This is just my opinion of course but I think if this PR is introducing state logging on load it should also include logging of state changes.

I considered it but it is IMO, too verbose, and not that useful, in the same vein that, for example sceneitem order changes aren't logged, only sceneitem order on load.

Would it perhaps be better to identify enabled or disabled filters just by adding (disabled) or (enabled) somewhere on the line? Or even just (disabled) for the disabled ones, with lack of it implying not-disabled?

That way no special knowledge is needed to read that information in the log, the filter lines are just self-explanatory.

Quite in the same vein, my goal was to preserve simplicity and keep verbosity at a minimum. I feel that the way it is is decently self explanatory, but am deffo open to other alternatives.

@prgmitchell
Copy link
Copy Markdown
Member

This is just my opinion of course but I think if this PR is introducing state logging on load it should also include logging of state changes.

I considered it but it is IMO, too verbose, and not that useful, in the same vein that, for example sceneitem order changes aren't logged, only sceneitem order on load.

I disagree with the comparison and don't think it would be very verbose unless someone is constantly enabling/disabling sources and filters. If the motivation for this change is to assist with support then having a way to determine the current state is more valuable then just knowing the initial state at load.

@Penwy Penwy force-pushed the log-item-enabled branch from 859b258 to 31b450d Compare August 14, 2024 22:39
@Penwy
Copy link
Copy Markdown
Contributor Author

Penwy commented Aug 14, 2024

This is just my opinion of course but I think if this PR is introducing state logging on load it should also include logging of state changes.

I considered it but it is IMO, too verbose, and not that useful, in the same vein that, for example sceneitem order changes aren't logged, only sceneitem order on load.

I disagree with the comparison and don't think it would be very verbose unless someone is constantly enabling/disabling sources and filters. If the motivation for this change is to assist with support then having a way to determine the current state is more valuable then just knowing the initial state at load.

After sleep and reconsideration, yeah, fair. Added.

@Penwy Penwy force-pushed the log-item-enabled branch 2 times, most recently from 52dca3c to e67bbde Compare August 15, 2024 00:35
@Penwy Penwy changed the title UI: Log sceneitem and filter enabled state on load UI: Log sceneitem and filter enabled Aug 15, 2024
@prgmitchell
Copy link
Copy Markdown
Member

I will also add that I agree with @alinsavix on using (disabled) for the disabled items only and logging nothing for something that is enabled.

@Penwy Penwy force-pushed the log-item-enabled branch from e67bbde to 4f954df Compare August 16, 2024 18:11
@Penwy
Copy link
Copy Markdown
Contributor Author

Penwy commented Aug 16, 2024

Done.
I still personally dislike the way it prints, but if you two agree on this, I'll bow to the majority.
example log here

@WizardCM WizardCM added the kind/feature Functionality or other elements that the project doesn't currently have. label Aug 17, 2024
@Penwy Penwy force-pushed the log-item-enabled branch from 44ec4db to 5ec38a2 Compare March 8, 2025 14:05
@Warchamp7
Copy link
Copy Markdown
Member

@Penwy Can you fix up the conflict on this?

@Warchamp7 Warchamp7 added this to the OBS Studio 32.2 milestone May 6, 2026
@RytoEX RytoEX requested review from Fenrirthviti and Warchamp7 May 8, 2026 20:31
Copy link
Copy Markdown
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.

Commit prefixes and PR prefix will need to be updated to "frontend: ".

Requested relevant reviews.

Comment thread frontend/components/SourceTreeItem.cpp Outdated
Comment thread frontend/components/SourceTreeItem.cpp Outdated
Comment thread frontend/components/VisibilityItemWidget.cpp Outdated
@Penwy Penwy force-pushed the log-item-enabled branch from 412f64e to 06059af Compare May 10, 2026 17:47
@Penwy Penwy changed the title UI: Log sceneitem and filter enabled frontend: Log sceneitem and filter enabled May 10, 2026
@Penwy
Copy link
Copy Markdown
Contributor Author

Penwy commented May 10, 2026

Upon revisiting it I realised this leaves one logging without that info :
When creating a new source or a sceneitem of an existing source through the "add source" dialog, the created sceneitem can be created disabled.
It is however impossible to log this in one line as the visibility is set after the creation of the sceneitem, and hence the logging of its creation.
Rather than add a second line of log for this, I think it is preferrable to not log the visibility status in that instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Functionality or other elements that the project doesn't currently have.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants