zanoni-mbdyn / blendyn

MBDyn (https://www.mbdyn.org/) graphical post-processor for blender (https://www.blender.org/)
GNU General Public License v2.0
40 stars 8 forks source link

Remove old Blender objects corresponding to removed nodes #10

Closed janga1997 closed 7 years ago

janga1997 commented 7 years ago

Fixed #9 I haven't tested, I will tomorrow.

I think I correctly implemented the part of removing objects from the scene. Although I have to still work out the part of removing the corresponding old nodes from the scene.

Removing all nodes is as simple as mbdyn.nodes.clear(), and similarly for the elements. But here I have to remove nodes from a specific list.

louisgag commented 7 years ago

Ok, let us know once you tested it. It wouldn't make sense to accept an untested pull request, would it?

janga1997 commented 7 years ago

Sure, there is a little more work on this PR, w.r.t. clearing nodes after removing the objects, that i wanted comments on. So, i thought it would be wise to post what i have done till now.

zanoni-mbdyn commented 7 years ago

One problem that I see immediately with your solution @janga1997 is that it assumes that in the moment the user re-loads the .log file, all the nodes/elements have a corresponding Blender object in the scene.

That might not be the case.

janga1997 commented 7 years ago

@zanoni-mbdyn I saw what you're referring to to when I was testing. Nodes/elems that don't have a corresponding Blender object seem to return an empty string '' for object name. So , I decided to just filter the obj_names list of empty strings instead of changing anything else, and hid the corresponding objects. I tested it with falling rigid body example, by adding another rigid body to it, and it does work.

I still have to add the dialog box we discussed about.

janga1997 commented 7 years ago

@louisgag I added the fix. And it works. You may also want to try out the hiding objects feature you requested about.

Although I still have to add a dialog box to warn the user.

janga1997 commented 7 years ago

@zanoni-mbdyn Is there a way to selectively delete some nodes. (either node by node, or a list of nodes) I understand that I can clear all the nodes with mbs.nodes.clear().

But I want to clear the nodes which the user has deleted (after choosing the delete option), because it raises a issue if I want to import a new results file again.

janga1997 commented 7 years ago

So, this feature is, I believe is 90% complete. File 1 - file with one single node File 2 - file with two nodes

  1. The option of hiding missing nodes works well, upon repetition too, i.e., repeatedly importing and animating with File1 and File 2, poses no issues.
  2. The option of deleting missing nodes works the first time, but upon repetition, causes an issue because after deletion, even though the object(s) is/are deleted from the scene, its corresponding node remains. Thus the previous comment.
louisgag commented 7 years ago

OK good, keep us updated :)

On 21-Mar-2017 07:57 PM, VSN Reddy Janga wrote:

So, this feature is, I believe 90% complete. File 1 - file with one single node File 2 - file with two nodes

  1. The option of hiding missing nodes works well, upon repetition too, i.e., repeatedly importing and animating with File1 and File 2, poses no issues.
  2. The option of deleting missing nodes works the first time, but upon repetition, causes an issue because after deletion, even though the object(s) is/are deleted from the scene, its corresponding node remains. Thus the previous comment.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zanoni-mbdyn/blendyn/pull/10#issuecomment-288183545, or mute the thread https://github.com/notifications/unsubscribe-auth/ALuOff8oBkzSzFSNdRT3NSxurboIjTfNks5roB2ygaJpZM4Mfy0L.

-- Louis Gagnon, Ph.D. Postdoctoral fellow Dipartimento di Scienze e Tecnologie Aerospaziali Politecnico di Milano +39 02 2399 8642 http://louisgagnon.com/research/

zanoni-mbdyn commented 7 years ago

@janga1997 yes there is a method to remove one single node from the collection. Let's say that you want to remove the node with index 2:

nodes = bpy.context.scene.mbdyn.nodes
nodes.remove(2)

Be careful though, because directly removing a node from the collection might cause problems to other entities that referred to that node. I'm thinking of elements (joints, beams, bodies, etc). They should be removed from the corresponding list too (they cannot be present in the MBDyn output since the related nodes have been removed).

Cheers Andrea

janga1997 commented 7 years ago

@louisgag @zanoni-mbdyn I circumvented the need to delete nodes/elems. I believe this feature is complete.

I've tried all possible combinations of deleting/hiding objects, with no errors. Do review at your leisure.

zanoni-mbdyn commented 7 years ago

@janga1997 it seems to me like a good solution. I've made just a few minor comments on the code. I will accept the PR when you have gone through them.

Cheers Andrea

janga1997 commented 7 years ago

@zanoni-mbdyn I couldn't find the comments?

zanoni-mbdyn commented 7 years ago

Sorry, my mistake. You should see them now.

janga1997 commented 7 years ago

@zanoni-mbdyn I added the changes. And if we think about it, the user would need to set the hide/delete option only after a previous simulation, so there is no problem of a default setting. He can set the option before proceeding to the second simulation. So I suppose that eliminates the need of an error message.

zanoni-mbdyn commented 7 years ago

Actually, I'd still warn the user that some of the nodes of the original model were not found after the last re-load of the .log file. It would be a harmless redundant information for the vast majority of the users, but sometimes it might be useful, for example if some mistake was made in generating a parametric model.

The message should be a warning, not an error, though.

janga1997 commented 7 years ago

@zanoni-mbdyn I added the warning report.

zanoni-mbdyn commented 7 years ago

Can you also move the Enum property from the to the MBDynImportPanel (in base.py), below the Load .log file and Load MBDyn labels buttons?

To me, that's a more logical position in the UI. We're almost done ;)

Cheers Andrea

janga1997 commented 7 years ago

@zanoni-mbdyn I've changed the location as you requested.

zanoni-mbdyn commented 7 years ago

Merged (with some minor modifications to the UI).