`ModelObserver #onPidChanged` callback docstring clarity

[DanRathbun]

SketchUp Ruby API Documentation Issue

Sketchup::ModelObserver#onPidChanged does not fire when the model changes.

There is no Model#persistent_id method. However, there IS a Model#guid method. Making changes to the model and saving causes a change in the guid, but a model observer #onPidChanged callback never fires.

Observed on SU2018 Pro.

Q: Under what situations would this observer callback fire ? ANSWERED BELOW

---

YARD docstring is easily misunderstood, see proposed changes in 3rd post.

[Eneroth3]

I don't think the GUID has anything to do with this observer. The model GUID is renewed for each operation, just like a definition GUID. onTransactionCommit should fire for each such change.

If I understand onPidChanged correctly it should fire when an entity within the model gets a new PID, not the model itself.

[DanRathbun]

Hmmm ... okay, perhaps I misread the docstring?

Could add an ("for an entity",) into the docstring, move the word "changes", and use "within" rather than "in", viz:

"The #onPidChanged method is invoked when a persistent id (for an entity,) changes within the model."

1. The words "persistent id" need to be linked to Sketchup::Entity#persistent_id where the list of entity classes that have pids are listed.

2. The following phrase "For example when entities are grouped." seems unneeded and does not add to the understanding. (ADD: Also there are non-drawingelement entities like pages that also get pids.)

3. Adding a note or "See Also" link to Sketchup::Model#find_entity_by_persistent_id() would be helpful.

[DanRathbun]

I am also confused as to the worth of this observer callback.

What would an observer be able to do when this callback fires?

• It gives an old pid and a new pid, but how would a coder use the pids to reference an actual object?

EDIT: To answer my own query, I couldn't find the method searching the API docs on "pid", but when I searched on just "id" I found it.

• Sketchup::Model#find_entity_by_persistent_id()

(I had expected to find it in the Sketchup::Entities class, and did a manual search of it's doc. It makes sense that it is an instance method of the Sketchup::Model.))

[DanRathbun]

class PidSpy < Sketchup::ModelObserver
  def onPidChanged(model, old_pid, new_pid)
    puts "onPidChanged: #{model}, #{old_pid} => #{new_pid}"
  end
end

Sketchup.active_model.add_observer(PidSpy.new)

As a test, I went to insert the "Car Sedan" dynamic component example from the "Components Sampler" folder, (into an empty model,) and got 2711 firings of this callback from ... onPidChanged: #<Sketchup::Model:0x0002881b1248c8>, 0 => 11173 ... all the way to ... onPidChanged: #<Sketchup::Model:0x0002881afcf590>, 0 => 13883 ... even before I had placed the instance of the car.

So all this was generated by adding the definition's entities (1677 edges, 490 faces, 18 instances, and 8 non-texture materials.) The 0 old pid doesn't say much except that this is a new entity, so it's a new pid assignment.

It seems beyond weird that the actual reference to the object whose pid changed (or was assigned) was not included in the callback parameters.

@thomthom, Any good reason why the coder has to call the search method instead of just supplying it as a parameter ?

[Eneroth3]

Regarding phrasing, changing "in model" to "within model" would probably make the method easier to understand.

I've never used PIDs myself but at a first glance it would sure make sense to pass the entity as a parameter.

[DanRathbun]

Regarding phrasing, changing "in model" to "within model" would probably make the method easier to understand.

Applied this suggestion to edit summary in 3rd post.

[thomthom]

I am also confused as to the worth of this observer callback.

What would an observer be able to do when this callback fires?

I agree - I wasn't happy about this either when it came up. But PIDs was added very late in the release cycle so changes were too late to cater for the API.

[thomthom]

Do I read this thread correctly in that the main issue here is improving the documentation?

[DanRathbun]

Well yes better docs would have answered what the callback was for.

But might as well add a FR for either an extra object reference arg, or an alternate callback.

I prefer the former as calling 2 callbacks is just not efficient, to try to solve the inefficiency of having to call the search method each time the callback fires. (SketchUp can prevent breaking changes by getting the arity of the observer's callback and adapting the parameter list for the call.)

[DanRathbun]

Do I read this thread correctly in that the main issue here is improving the documentation?

Leaving this issue as a documentation issue, I have opened a separate FR issue for persistent_id observer callbacks.

Retitled issue ...