ramagottfried / symbolist

A library for graphic/symbolic score editing
15 stars 3 forks source link

setOneSymbol missing connection to updateExistingScoreSymbol #109

Closed ramagottfried closed 6 years ago

ramagottfried commented 6 years ago

Preface: this is not a big deal, but we should go through and check to see which things weren't merged from the master. It looks like my hot-fix commit from March 12 wasn't added to the current active dev branch. I somewhat forget which things I added, but it would be good to make a comparison eventually.

setOneSymbol should set the symbol in the score. The current behavior in the master branch is that if a new symbol is sent in by the user which matches an existing id, the new symbol will replace the old one. This functionality makes it possible to move certain symbols dynamically / programmatically.

I see now that the application-restructuring branch didn't actually merge the master branch from the March 12 updates -- where I had added updateExistingScoreSymbol.

see the setOneSymbol update here: https://github.com/ramagottfried/symbolist/blob/fd5c63d855a4ddf3bac264cc3b7d3316c6c05b59/Source/SymbolistHandler.cpp#L178

and the addition of Score::updateExistingScoreSymbol: https://github.com/ramagottfried/symbolist/blob/fd5c63d855a4ddf3bac264cc3b7d3316c6c05b59/Source/Score.cpp#L117

The following code from Score::addSymbol breaks this functionality:

    auto iteratorToSymbol = find_if(score_symbols.begin(),
                                    score_symbols.end(),
                                    [symbol](unique_ptr<Symbol>& ptrToSymbol) {
                                        return ptrToSymbol.get() == symbol;
                                    });

    // If symbol exists in score, then return its reference.
    if (iteratorToSymbol != score_symbols.end())
        return (*iteratorToSymbol).get();

if the symbol exists, we should return the reference, but union it with the incoming symbol. I just updated the addSymbol function to account for this:

// Calls symbol's empty constructor if reference is NULL.
    if (symbol == NULL)
    {
        score_symbols.push_back(unique_ptr<Symbol>(new Symbol()));
        return score_symbols.back().get();
    }

    // make copy and add to symbol vector
    score_symbols.push_back(unique_ptr<Symbol>(new Symbol(*symbol)));

    /* If symbol id exists in score, then union the incoming values with the current values
     * and return the symbol reference.
     */
    string id = symbol->getID();
    auto iteratorToSymbol = find_if(score_symbols.begin(),
                                    score_symbols.end(),
                                    [id]( unique_ptr<Symbol>& ptrToSymbol )
                                    {
                                        return ptrToSymbol->getID() == id;
                                    });

    if (iteratorToSymbol != score_symbols.end())
    {
        (*iteratorToSymbol)->unionWith( symbol, true );
        return (*iteratorToSymbol).get();
    }

... but it's not completely working yet, I can take a look soon and see if I can fix it, I the input format from Max might need some updating also.