"changes" trigger is not firing correctly

[Tristan Croll]

def print_structure_changes(trigger_name, changes):
    print('Structure changes: {}'.format(changes[1].atom_reasons()))

def print_global_changes(trigger_name, changes):
    print('Global changes: {}'.format(changes.atom_reasons()))

m = session.models.list()[0]
from chimerax.atomic import get_triggers

h1 = m.triggers.add_handler('changes', print_structure_changes)
h2 = get_triggers(session).add_handler('changes', print_global_changes)

m.atoms.coords += 1
# Nothing prints

session.change_tracker.changes
# Now the following prints to the log:
# Global changes: ['coord changed']
# Structure changes: ['coord changed']
# Global changes: []

m.atoms.coords += 1
m.atoms.displays = True
# Nothing prints

session.change_tracker.changes
# Global changes: ['coord changed', 'display changed', 'hide changed']
Structure changes: ['coord changed', 'display changed', 'hide changed']
# Global changes: []

I'm quite heavily reliant on the 'changes' trigger, so this is a blocker for me.

[Tristan Croll]

Ah - I see it's my fault. In ChangeTracker.h, changed() is checking for changes in _global_type_array, which of course is now empty until get_global_changes() is called - so it always returns false, meaning atomic.check_for_changes(session) returns without doing anything.

I think this replacement in ChangeTracker.h (line 206) should do the trick (haven't built it yet, though):

    bool  changed() const {
        for (auto & s_changes: _structure_type_changes) {
            if (s_changes.second.changed())
                return true;
        }
        return false;
    }

[Tristan Croll]

Sorry, make that:

    bool  changed() const {
        for (auto & s_changes: _structure_type_changes) {
            auto &structure_changes = s_changes.second;
            for (auto &c: structure_changes)
                if (c.changed())
                    return true;
        }
        return false;
    }

[Tristan Croll]

I've rebuilt and confirmed that this corrects the bug.

[Eric Pettersen]

Tristan's proposed fix does indeed fix the problem.