npruehs / tome-editor

Tome is a generic data editor for games supporting arbitrary input and output formats.
GNU Lesser General Public License v3.0
36 stars 7 forks source link

Creating fields sometimes removes all fields from category records. #195

Closed tylerhasman closed 2 years ago

tylerhasman commented 2 years ago

I've had it happen a few times while working where I'll create a new field and one of my category records will become totally blank. This ends up deleting all the data in the children records because they all inherit the category's fields. I'm not sure the exact steps to reproduce this because it seems to happen sometimes and not others. Luckily I've been using version control so when this happens I just revert but it's a little frustrating.

Not sure if this is related but, when I duplicated a child record and renamed it, it removed the parents fields completely.

I've managed to create some data where the bug is reproducible, if you'd like I can share it. When I edit the name of one record, an entirely unrelated record will have all its fields removed.

tylerhasman commented 2 years ago

I think this is related to #186

tylerhasman commented 2 years ago

I've forked this repo and done some detective work, the issue seems to be caused by sorting of the record tree.

Specifically this part in recordscontroller.cpp


    bool needsSorting = oldDisplayName != displayName;

    if (needsSorting)
    {
        for (RecordSetList::iterator it = this->model->begin();
             it != this->model->end();
             ++it)
        {
            std::sort((*it).records.begin(), (*it).records.end(), recordLessThanDisplayName);
        }
    }
tylerhasman commented 2 years ago

Ok it looks like it has something to do with how things are being sorted for sure. Picking diffierent names for the new record causes different records to be altered as a side-effect.

tylerhasman commented 2 years ago

Ok I've found the issue.

setRecordDisplayName(newId, newDisplayName);

causes the record list to be changed which in turn causes

Record* record = this->getRecordById(newId);

in RecordsController::updateRecord to now point to the wrong record. This should be a simple fix now.

tylerhasman commented 2 years ago

Fixed in #196