defcon icon indicating copy to clipboard operation
defcon copied to clipboard

order of registry posibilities

Open typemytype opened this issue 8 years ago • 3 comments

when an object is self observing it is subscribed to all notifications, this makes sense in order to destroy representation factories.

But the order of calling observers puts the self observing as third option. see https://github.com/typesupply/defcon/blob/master/Lib/defcon/tools/notifications.py#L146-L151

This causes issues when a factories has no destructive notifications (aka ".Changed" notification) as this is being called afterwards.

a small example to demonstrate this issue.

I don't have any idea to push self observing notifications upfront. The factories could add all possible notifications but that does not feels right, especially with heavy calculating factories.

from defcon import Font, Kerning, registerRepresentationFactory

count = 0

def testFactory(kerning):
    print "    *** create a new repr ***"
    global count
    count += 1
    return count

registerRepresentationFactory(Kerning, "test", testFactory, destructiveNotifications=None)


class TestKerning(Kerning):
    
    def selfNotificationCallback(self, notification):
        print "    ----> self notification", notification.name
        super(TestKerning, self).selfNotificationCallback(notification)
        print "    ----> self notification done", notification.name


class Test(object):

    def __init__(self):
        f = Font(kerningClass=TestKerning)
        f.kerning.addObserver(self, "_kerningChanged", "Kerning.Changed")
        print "testing kerning:"
        print "initiate", f.kerning.getRepresentation("test")
        f.kerning[("a", "b")] = 10
        print "done", f.kerning.getRepresentation("test")

        
    def _kerningChanged(self, notification):
        print "    kerning changed", notification.name
        print "    %s" % notification.object.getRepresentation("test"), "(should build a new representation but the factory is not destroyed yet...)"
    
Test()
testing kerning:
initiate     *** create a new repr ***
1
    ----> self notification Kerning.PairSet
    ----> self notification done Kerning.PairSet
    kerning changed Kerning.Changed
    1 (should build a new representation but the factory is not destroyed yet...)
    ----> self notification Kerning.Changed
    ----> self notification done Kerning.Changed
done     *** create a new repr ***
2

typemytype avatar Feb 15 '17 13:02 typemytype

I wondering why the order is going from most specific -> least specific?

typemytype avatar Feb 15 '17 13:02 typemytype

I just hit this issue with a SelectionChanged notification sent from contour, which the glyph watches then sends its own Glyph.SelectionChanged that destroys the representation.
This problem affects the call to destroyRepresentations in BaseObject since it makes a subscription to all the notifications it sends. This means destructors aren't run first when they absolutely should.

@typesupply Could you explain the rationale for the order of notifications:

I wondering why the order is going from most specific -> least specific?

I expect simply reversing that order should fix this issue.

Edit: Although reversing the order fixes the issue with destroyRepresentations, I think destructors should have some special rooting in NotificationCenter so they're guaranteed to be called first.

adrientetar avatar Mar 26 '17 12:03 adrientetar

Could you explain the rationale for the order of notifications:

I don't remember. That # most specific -> least specific note is a sign that I had some reason, but I don't know what it was. I'm 100% okay with changing the order and seeing if it breaks anything. I'm also okay with some sort of special destructive pre-run if necessary.

typesupply avatar Mar 26 '17 17:03 typesupply