Pull request (lk.lkaz)

[lk-lkaz]

Dear Triangle717:

Hello, my screen name is "lk.lkaz". I am an user of your "LDR Importer" Blender addon.
Today, I am writing this message for two purposes.

1. Report a bug fix, and my idea for faster importing.

2. Ask for permission to upload my mod program to my brickshelf page.

3. Report a bug and ideas
   First, I found a bug while importing "2555.dat". It separates into 2 objects.
   I figured out an idea for fix it. Search "lk_mod_1" to see what I changed.
   Second, I modified the way of creating mesh and object in LDrawFile() for faster importing.
   Search "lk_mod_2", "lk_mod_3", and "lk_mod_4" to see what I changed.
   If you like my modifications, would you accept them for next release, please?

4. Ask for permission
   I think that the last modification "lk_mod_5" is not suitable for common use.
   It is made for my specific perpose. I am now making a re-exporter program.
   My plan is like this -->
   a) Import a LDraw model into Blender.
   b) Set location by rigid_body simulation.
   c) Export a new LDraw model with simulated location.
   d) Render by POV-Ray with LGEO library.
   When I finished the work, I want to publish the exporter program with the paired importer program.
   Is it possible to upload my mod program(based on your v1.1.0) to my brickshelf page? Or upload it to github as a new branch?

Sincerely,
lk.lkaz

[JoshTheDerf]

It looks very nice, thank you very much for the time you've put in to this. I don't have time to test all of the revisions ATM, @le717 can better answer your questions.

Just a few suggestions for future pull requests (I don't mean to be picky):

• Try to name your lk_mod_#'s something more descriptive and put them in separate branches. Although it's more work, it makes reading through a pull request and rebasing it much easier.
• Don't put comments related to your GitHub pull requests in the code. Doing so makes it much more confusing if other people who have not seen the referenced pull request.

Thanks again for your hard work!

[lk-lkaz]

Thank you for the quick response. Yes, I struggled on it for these three manthes X)
And sorry for wrong useage of GitHub pull requests. I must learn more about GitHub.

[rioforce]

Not bad, there are a few features in there like linking mesh and setting the origin that will help a lot. The only problem is, they are always default, which makes the Original LDraw Mesh function useless. The linked mesh should be in a separate check box, and the origin to mass should replace the Origin to Geometry feature we already have. The apply scale isn't needed, because the scale is already applied on the mesh when it imports. I'm looking into fixing a few of these issues now.

Like @tribex said, each major change you make to the script, you should submit a new Commit on your forked Git Repo. It makes it a lot easier to figure out what you changed and for what reason. :)

[Banbury]

I want to add, that LDR-Importer is open source. As long as you adhere to the license, you don't need a permission from anyone, to use the code and publish it as part of another project.

[le717]

First off, thank you for the pull request! :D Don't feel too bad about messing it up. We can work through that. ;) If you haven't already, please take the time to read the contributing guidelines on the wiki. We are not that strict; they are in place to help, not hinder, you in your coding. 😉

Yes, as @Banbury said, as long as you adhere to the GPL v2, you are free to create a fork. See http://choosealicense.com/licenses/gpl-v2/ for an overview. ;)

That being said, there are changes in here I think we can merge. I also have some questions. However, I'm reading homework right now, so I'll ask them later. ;)

[lk-lkaz]

rioforce:
Thank you for the precise check and advices. I will try to fix them ;)

Banbury, le717:
I am now understand the license. I can concentrate on my project.
Thank you!

[le717]

Hello @lk-lkaz!

First off, thank you for the time and effort you put into this pull request. I can tell you performed a lot of work, and I am most thankful for that. 😄

After reviewing the code and the changes made, I regret to say that I will not be merging it as-is. Some of the code added or removed can be done more simply or better using another method. However, there are parts that would be beneficial to the LDR Importer. So while your script here will not be merged in the current form, parts of it will be added to the script in other commits by myself and the rest of the development team. You will be credited will the initial patch in the change log and your name added to the list of contributors, thus your contribution will not go unnoticed. I do this because we had not had the ideas you did in this patch. It is common for patches to be conceived by one person but reimplemented or improved by another person. Although that original person may not be credited in some projects, I give credit where credit is due at all times, and for that I am adding your name. 😃

Again, thank you for your contribution and interest in the LDR Importer. I hope you have found mush use from it. :) I am leaving this issue open for now, so please do not close it. 😉 If you have any questions, find a bug, or have another contribution or feature idea, feel free to open a new issue. :) Again, thank you.

-le717

[lk-lkaz]

le717:
Thank you for reviewing and your polite response. I am honored to be able to contribute even a little.
While reading the program, I was very impressed with its elaborate methods. Please keep on great work ;)