Doubts about 'add_observer'

Examining the code, I can see that adding an observer is quite efficient (Around logarithmic time). Adding the same observer to the same entity multiple times is safe, though not as efficient as it should be due to historical reasons. It will essentially detect that the given observer is already observing the target entity. If it does, it removes the internal observer first and creates a new one. To the Ruby side, it seems like nothing has happened, but the observer gets moved to the end of the queue. So @DanRathbun is right. But I’m not sure why this information would be useful to the API user. (There might be a reason I’m not thinking of). But it’s not wise from an API designer’s perspective to make such implementation details into API contracts. That severely limits our future ability to change these internal details and improve them later.

The current implementation of removing an observer is far less efficient. So I can believe that doing a batch remove on a huge model would take a long time. Looks like we have room for improvement there.

For better performance, it is a good idea to reduce the number of observers used. So it is better to observe entities at higher level, e.g. groups or instances rather than faces and edges, for instance.

And the reason there isn’t a way to get a list of all observers is that all plugins are currently running inside the same Ruby environment and we don’t have a mechanism to differentiate which plugin owns which observer. We wouldn’t want to hand you references to someone else’s observers, potentially letting you remove something you didn’t add.

I hope these help. Please consider these just as the current state of the code and not promises about how they will behave in the future.

3 Likes

Just have a weird memory.

Along with that old discussion, I (and others participating) had concluded and requested that a query method be implemented so that we could ask an object if it was being “watched” by a certain observer. Without such a query method there is need for knowing the facts of the callback queue behavior. But once a coder knows, they would perhaps want a query method to first ask an object if their observer was already attached, and if so skip the add step.

object.add_observer(@spy) unless object.watched_by?(@spy)

If they are aware of the queue, they may in some cases wish to keep the observer’s current position early in the queue, rather than move it to the end.

This scenario is not hypothetical. There is one commercial extension I did work on that was written (by the original author) in such a way that it’s observer needed to first before any others. (NDA prevents naming names.)

Well I understand your point of view, but I am not from that school. APIs do not exist to make the designers happy. They are a product that should make the API end users happy.

I’m also not of the opinion that once implemented features are set in stone. Even the SketchUp API underwent an observer overhaul that broke many plugins and most of the native tool observers onToolStateChanged() callbacks are now worthless. (Ie, all the states are saved up “in a bundle” and then called en masse at the end of the tools state cycle.) So basically I believe in making things better when it is necessary, even if it is a “breaking change.”

Specifically, in this case, I’d think that there would always be some kind of FIFO observer queue. How else can the engine know what observer’s callbacks to call?

What would it hurt mentioning the queue in the docs, or implementing at the very least a boolean query method to determine if an observer object was already attached ?

I don’t think I ever asked for shift(), unshift(), push() or pop() methods for observer queues as that would open Pandora’s box (ie, bad extensions causing havoc with other coder’s extensions.)

I agree with this, but unfortunately the problem is that not everything is correctly handled by EntitiesObserver.

@tt_su highlighted it here:

Thanks for everybody, now I see that I need to change my approach to solving push/pull problem.

1 Like

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.