frontend: Fix handling scene remove signal from not main canvas

[exeldro]

Description

Fix handling scene remove signal from not main canvas

Motivation and Context

OBS_FRONTEND_EVENT_SCENE_LIST_CHANGED was triggered when a scene on not main canvas was removed

How Has This Been Tested?

On windows 11 by removing a scene for an extra canvas.

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.

[Warchamp7]

I think OBSBasic::RemoveScene should be updated instead to only fire the OBS_FRONTEND_EVENT_SCENE_LIST_CHANGED event if an item was removed from the scenes list. There is additional functionality in there like SaveProject() that should probably still fire anytime a source is removed.

[exeldro]

Moved the check to OBSBasic::RemoveScene and made sure SaveProject() is called for not EPHEMERAL canvases

[PatTheMav]

The change itself looks correct, but I feel like we could use this opportunity to freshen up the implementation a bit:

void OBSBasic::RemoveScene(OBSSource source)
{
    OBSCanvasAutoRelease canvas = obs_source_get_canvas(source);
    
    if (!canvas) {
        assert("OBSBasic::RemoveScene: Source passed to function has no associated canvas.");
        return;
    }
    
    uint32_t canvasFlags = obs_canvas_get_flags(canvas);
    
    bool isMainCanvas = (canvasFlags & MAIN);
    bool isEphemeralCanvas = (canvasFlags & EPHEMERAL);

    if (isMainCanvas) {
        // Pointer lifetime coupled to strong reference to "source"
        const char* sourceUuid = obs_source_get_uuid(source);
        
        if (!sourceUuid) {
            assert("OBSBasic::RemoveScene: Source passed to function has no UUID.");
            return;
        }
        std::string_view lhs {sourceUuid};
        
        SceneTree *scenes = ui->scenes;
        
        auto findMatchingListItemRow = [](QListWidget *list, std::string_view uuid) -> std::optional<int> {
            if (!list || uuid.empty()) {
                return std::nullopt;
            }
            
            int numListItems = list->count();
            
            for (int i = 0; i < numListItems; ++i) {
                QListWidgetItem *item = list->item(i);
                
                OBSScene sceneItem = GetOBSRef<OBSScene>(item);
                OBSSource sourceItem = obs_scene_get_source(sceneItem);
                const char *itemUuidPtr = obs_source_get_uuid(sourceItem);
                
                if (!itemUuidPtr) {
                    continue;
                }
                
                std::string_view itemUuid {itemUuidPtr};
                
                if (uuid != itemUuid) {
                    continue;
                }
                
                return {i};
            }
            
            return std::nullopt;
        };
        
        std::optional<int> match = findMatchingListItemRow(scenes, sourceUuid);
        
        if (match) {
            int matchedItemRow = match.value();
            int currentItemRow = scenes->currentRow();
            
            if (matchedItemRow == currentItemRow) {
                ui->sources->Clear();
            }
            
            QListWidgetItem *matchedItem = scenes->takeItem(matchedItemRow);
            delete matchedItem;
        }
    }
    
    if (!isEphemeralCanvas) {
        SaveProject();
    }
    
    if (isMainCanvas) {
        if (!disableSaving) {
            blog(LOG_INFO, "User Removed scene '%s'", obs_source_get_name(source));
            
            OBSProjector::UpdateMultiviewProjectors();
        }
        
        OnEvent(OBS_FRONTEND_EVENT_SCENE_LIST_CHANGED);
    }
}

@Warchamp7 please give this a whirl and check if I missed something. Scene identity is checked using the UUID and list item removal is based on the underlying data model index and not memory addresses.

[Warchamp7]

Both of the above unfortunately run into the same problem I just dealt with on obs-websockets. The global source_remove signal can arrive too late for obs_source_get_canvas to succeed, as it will have already been detached from it's canvas by then. Thus any introspection and conditions based on canvas in response to the global source_remove will not work.

We would have to connect to the canvas-specific source_remove signal for that, and that's out of scope here and a longer term fix.

In the short term, I think we have to settle for simply checking if we removed something from the list. A scene from another canvas should never end up in the ui->scenes list so if nothing was removed, no frontend event.

[exeldro]

Applied some of the changes suggested by @PatTheMav .
Did early return when the source has no canvas, but no assert as this can happen in multiple ways at the moment in OBS
Used bool isMainCanvas and bool isEphemeralCanvas
I do not see any advantage in comparing uuid instead of pointer at the moment.
Added the takeItem before the delete.

As @Warchamp7 suggested now only trigger OBS_FRONTEND_EVENT_SCENE_LIST_CHANGED when an item is removed from the scene list

@Warchamp7 in what case can the global source_remove signal arrive too late for obs_source_get_canvas? In my tests by removing scenes from the list using the delete key it worked as expected.

[Warchamp7]

@Warchamp7 in what case can the global source_remove signal arrive too late for obs_source_get_canvas? In my tests by removing scenes from the list using the delete key it worked as expected.

Websockets or a plugin