SketchUp / api-issue-tracker

Public issue tracker for the SketchUp and LayOut's APIs
https://developer.sketchup.com/
38 stars 10 forks source link

allow set persistent_id upon element creation #361

Open kengey opened 5 years ago

kengey commented 5 years ago

It would be great if we could set a persistent_id upon the creation of elements that have a persistent id. Right now this is somewhat the only missing thing that allows us for total serialization and deserialization of SketchUp models. I could imagine the methods that create entities allow for an extra persistent_id argument. This could throw an error if an id is already used by another element. If nil is given it just works as it currently does.

This would allow for data transfer between .skp files without passing around .skp files as a whole.

thomthom commented 5 years ago

What about the scenario of the PID already existing? Raise an error?

kengey commented 5 years ago

What about the scenario of the PID already existing? Raise an error?

exact, as I wrote:

This could throw an error if an id is already used by another element. If nil is given it just works as it currently does.

thomthom commented 5 years ago

Ah - sorry. Blitzing quickly over my backlog of message.

DanRathbun commented 5 years ago

This is highly unusual. PIDs are generated by the API, not set by users or coders.

Adding an argument to Ruby existing methods (constructors included) might break all the code in the wild, unless the method was designed from the beginning to handle varying number of parameters, or say extra named hash arguments. It would need definately to be an optional argument.

This would allow for data transfer between .skp files without passing around .skp files as a whole.

What are you trying avoid? Passing materials, styles, layers, etc. All the "fluff" from an external (skp) definition ? If it doesn't have the same resource layer assignments (materials, styles, scenes, layers, etc.) then it is not the same object as such requires a new PID (generated by the API.)

What is a specific use case ?

For lower level elements would an API clone and/or dup instance method overrides be more Rubyish and safer ? These could be used by an API factory cloning method. Ie ... assume a Mac MDI scenario ...

model1.entities.grep(SketchUp::Face).each do |face1|
  model2.entities.add_clone( face1 ) # calls face1.clone internally
end

On the PC, We'd have to use a virtual Geometry Input helper collection to hold cloned objects until the second "target" model was loaded.

DanRathbun commented 5 years ago

For the C API, things are a bit different. Two models can be loaded into memory at the same time. It makes more sense that a definition could be copied in memory from one model dB to the other, without going through a temporary skp file written to disk.

kengey commented 5 years ago

This is highly unusual. PIDs are generated by the API, not set by users or coders.

Well, as an end user I don't really care no, but as a coder there are scenarios I do care about the persistent id that gets assigned to the entities I create. Persistent ids allow us to make references to specific entities from our own code.

To get to the point. Imagine I would want to make an app that allows real-time collaboration between multiple persons on remote locations. Ideally I would have one central model and changes get pushed to that central model and then recreated in the local models the collaborators have currently open. These models needs to be identical, right up to the persistent id.

kengey commented 5 years ago

To continue on the why, because an end users doesn't really care much of course. But extensions rely on those persistent ids.

DanRathbun commented 5 years ago

I just see issues if a code were to be allowed to send any argument as a PID. Something this important could crash the application or corrupt the model if it isn't correct.

Now similar to what I was thinking above, if the API(s) have a way of transferring the PIDs from one object to another directly, that may be "safe". Ie, ...

entity.set_pid_from(entity_responding_to_persistent_id_of_the_same_class)
jeroentheuns commented 5 years ago

I consider this thread really crucial and interesting. Looks like there are two important points of view ; seen by from PID's low level implementation and design intentions and perceived from the 'outside world' as an essential thing missing needed to build a foundation for scenarios allowing SU to venture into proper versioning systems (tracking model changes and history), proper collaboration scenarios, model variants, linking subsets of models (think splitting out parts of the model while tracking links to the original), being able to better link Layout and SU, and I could go on and on here, .... Now I think I understand both @DanRathbun and @kengey on this. Dan is most likely absolutely right in describing this as highly unusual and more so plain dangerous. On the other hand I am 1000% with Kenny on the extreme potential laying ahead if we could start implementing al these new use cases spinning off the concept of better tracking what are (or used to be) the same SU entities. I think there is indeed a technical mismatch between what specifications PID's were designed and implemented for and the huge potential explosion of new value for the SU community as a whole that @kengey is touching upon here. So I would strongly suggest to elevate this 'issue' way above just checking against current PID's situation and bring this thread to the table of whoever is involved in the future path for SU low level architecture. Thinking Doug, Yin, Bughra,.... (sorry if I misspelled and forgot names here). Would love to be involved on this obviously.

jeroentheuns commented 5 years ago

related to ComponentDefinition#guid= #278

DanRathbun commented 3 years ago

Revisiting this request issue again today ... another fact dawned upon me.

This is that the API documentation declares a contract thus ...

_The #persistentid method is used to retrieve a unique persistent id assigned to an entity. The persistent id [is] persistent between sessions.

So 2 guarantees here ... uniqueness and persistent between sessions.

Allowing persistent ids to be assigned would not make the contract enforceable.

In my opinion, if a coder needs to assign IDs, they can do so using a dictionary and an ID attribute that their code (or some Rubygem) generates.

kengey commented 3 years ago

Allowing persistent ids to be assigned would not make the contract enforceable. It can still be enforceable:

What about the scenario of the PID already existing? Raise an error?

exact, as I wrote:

This could throw an error if an id is already used by another element. If nil is given it just works as it currently does.

If the creational methods would allow to also pass a PID and that PID already exists in the model, it could just throw an error. And if it does not exist yet then I do not see the issue.

Again, my goal of being able to assign a PID is exact duplication of a remote model. Right now I can recreate geometry and attributes, but not PIDs. And that is a problem for other tools that rely on PIDs being there. Imagine a collaboration platform where users use 1 extension to sync their models (the one I want to achieve) and another to collaborate on assigning a schedule to their SketchUp model to do some 4D planning. That planning extension might use PIDs of element to store which task is assigned to which element.

Now user A makes changes (adds a wall and links that will to a task in the scheduling extension). User B, which is a team member of user A with the same extensions installed fetches those changes into his model by using the SYNC extension. All looks great, except that for user B the newly created wall will not be linked to the schedule task user A also created, because the PID of the wall is not the same for user B as it is for user A.

DanRathbun commented 3 years ago

If the creational methods would allow to also pass a PID ...

It is not this simple. They use factory methods that are scattered around the API's collection classes. Adding another argument can cause backward compatibility issues. (The API methods in older SketchUp versions are not expecting extra arguments and would likely raise ArgumentError exceptions.)

And then there are still some Entity types that do not yet support persistent id. This is also true of all the model's collection classes (including Attribute Dictionaries.)

And there are some, (Vertex and Style for examples,) that do, but do not have factory creation methods.


Again, my goal of being able to assign a PID is exact duplication of a remote model. Right now I can recreate geometry and attributes, but not PIDs.

I understand what you basically desire (the goal .. ie, model syncing.) I disagree that geometric recreation should be the solution.

This is basically the Xref scenario that other coders have struggled with. Julia has at least 1 extension in this realm and has several API feature requests open to make these kinds of extensions better.

thomthom commented 3 years ago

Again, my goal of being able to assign a PID is exact duplication of a remote model. Right now I can recreate geometry and attributes, but not PIDs. And that is a problem for other tools that rely on PIDs being there. Imagine a collaboration platform where users use 1 extension to sync their models (the one I want to achieve) and another to collaborate on assigning a schedule to their SketchUp model to do some 4D planning. That planning extension might use PIDs of element to store which task is assigned to which element.

What about model GUID and/or instance/definition GUIDs? Are they not relevant in the same way as PIDs?

DanRathbun commented 3 years ago

I am also thinking that perhaps the Model#import method might get a named option to preserve PIDs, rather than implement direct PID assignment all over the API.