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

UI: Fix remigrating scene collections from absolute to relative #11630

Open
wants to merge 1 commit into
base: release/31.0
Choose a base branch
from

Conversation

derrod
Copy link
Member

@derrod derrod commented Dec 14, 2024

Description

Fixes resetting scene collection base resolution to work again.

Motivation and Context

This was broken in #11055

How Has This Been Tested?

Locally.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

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 Bug Fix Non-breaking change which fixes an issue label Dec 14, 2024
@WizardCM WizardCM added this to the OBS Studio 31 milestone Dec 15, 2024
@Lain-B
Copy link
Collaborator

Lain-B commented Dec 15, 2024

If we want to put this on a hotfix, we can merge this directly to a release branch if needed and not master, and then redo this for master after the UI changes are merged.

@Warchamp7
Copy link
Member

If we want to put this on a hotfix, we can merge this directly to a release branch if needed and not master, and then redo this for master after the UI changes are merged.

We should try to get this out in a hotfix for sure

@derrod derrod changed the base branch from master to release/31.0 December 17, 2024 01:10
@RytoEX RytoEX requested a review from Warchamp7 December 18, 2024 01:26
@PatTheMav
Copy link
Member

PatTheMav commented Dec 27, 2024

I'm fine with fixing it on the release branch like this to get the issue fixed, but on master we need to find a different way. Adding a whole argument to ActivateSceneCollection that is not used in the vast majority of scenarios and thus requires to be set to false by default more or less telegraphs that it's not the correct way to do this.

So instead of carrying a remigrate flag around that is forwarded from nested function call to nested function call and only ever really processed by OBSBasic::LoadData, the requirement to remigrate should be part of the data that is loaded - you mark the data as "stale", which can then be read and detected by LoadData which then dutifully does the remigration.

Because ultimately no other method but OBSBasic::LoadData cares about this detail, so this detail should not leak into the internal API and function signatures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants