frontend: Only saved scene sources from frontend-managed canvases #12681
Conversation
WizardCM
added
Bug Fix
| #define DESKTOP_AUDIO_1 "DesktopAudioDevice1" | ||
| #define DESKTOP_AUDIO_2 "DesktopAudioDevice2" | ||
| #define AUX_AUDIO_1 "AuxAudioDevice1" | ||
| #define AUX_AUDIO_2 "AuxAudioDevice2" | ||
| #define AUX_AUDIO_3 "AuxAudioDevice3" | ||
| #define AUX_AUDIO_4 "AuxAudioDevice4" |
There was a problem hiding this comment.
If we're changing this, we might as well make those proper static constexpr std::string_views and update SaveAudioDevice appropriately, moving it into the anonymous namespace instead of using the static storage identifier, and only reaching into the char pointer at the boundary to the C API.
| obs_data_t *saveData = obs_data_create(); | ||
| OBSDataAutoRelease saveData = obs_data_create(); | ||
|
|
||
| /* Scene collection name */ |
There was a problem hiding this comment.
Use C++ comment style in C++ source code. The PR seems to mix both styles.
There was a problem hiding this comment.
Old habits die hard!
|
|
||
| /* -------------------------------- */ | ||
| /* save group sources separately */ | ||
| // We're saving groups separately ensures they won't be loaded in older versions. |
There was a problem hiding this comment.
| // We're saving groups separately ensures they won't be loaded in older versions. | |
| // Saving groups separately ensures they won't be loaded in older versions. |
| /* -------------------------------- */ | ||
| /* save group sources separately */ | ||
| // We're saving groups separately ensures they won't be loaded in older versions. | ||
| // ToDo: Get rid of this at some point. Groups were introduced in 22.0 |
There was a problem hiding this comment.
| // ToDo: Get rid of this at some point. Groups were introduced in 22.0 | |
| // TODO: Get rid of this at some point. Groups were introduced in 22.0 |
All caps should be most compatible with language servers and IDEs.
| obs_data_set_array(saveData, "transitions", transitions); | ||
| obs_data_set_array(saveData, "saved_projectors", savedProjectorList); | ||
| obs_data_set_array(saveData, "canvases", savedCanvases); | ||
| // Iterate over our additional canvases (if any), save their scenes and groups. |
There was a problem hiding this comment.
Why is there a distinction in code between the "main" canvas and "additional canvases"? Wouldn't it be simpler to iterate over all canvases and save them (if they are eligible for saving), as that would include the main canvas by default anyway?
There was a problem hiding this comment.
The distinction is made because the primary/main canvas always exists and is not itself part of the frontend's canvases list (at least, not yet).
And we only iterate that list rather than all canvases because we only want to save the scenes associated with frontend-managed canvases (i.e. ones that were created via the frontend API and are tracked in canvases), not any other ones that may have been created with libobs directly, e.g. by third-party plugins.
There was a problem hiding this comment.
Right so the "main" canvas still represents "the" scene collection, and canvases exist on top of that. Otherwise we'd need a "scene collection v3" that is built on top of a list of canvases as its "root" object?
| using sourcesAndGroups_t = decltype(sourcesAndGroups); | ||
|
|
||
| /* -------------------------------- */ | ||
| auto sceneSaveCallback = [](void *param, obs_source_t *source) -> bool { |
There was a problem hiding this comment.
It's not really a "save" callback, because it doesn't save anything. It only gets source and group data, so exportSceneItemsCallback would be more appropriate IMO.
3 participants
Description
Changes the saving logic a bit so that scenes from canvases not managed by the frontend won't get saved.
Also refactors the saving a bit since the separate
GenerateSaveData()seems unnecessary.Motivation and Context
The intention was that the frontend should only save and load canvases it manages (i.e. created via the frontend API). Likewise it shouldn't save any scenes on canvases that aren't frontend-managed or ephemeral.
This problem was reported on Discord.
How Has This Been Tested?
Saved and loaded some scene collections.
Types of changes
Checklist: