Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

order of registry posibilities #102

Open
typemytype opened this issue Feb 15, 2017 · 3 comments
Open

order of registry posibilities #102

typemytype opened this issue Feb 15, 2017 · 3 comments

Comments

@typemytype
Copy link
Member

typemytype commented Feb 15, 2017

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
Copy link
Member Author

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

@adrientetar
Copy link
Contributor

adrientetar commented Mar 26, 2017

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.

@typesupply
Copy link
Member

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.

typemytype referenced this issue Apr 26, 2017
similar as other classes in the glyph (contourClass, pointClass,
anchorClass, guidelineClass)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants