New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed bugs and added an experimental prefab export operator #49

Merged

Cervator merged 7 commits into MovingBlocks:master from kartikey0303:master

Jul 25, 2017

Contributor

kartikey0303 commented Jul 12, 2017

Fixed the following bugs:

Bounding box
File directory not being created
Matrix error is also not present.

Prefab exporter:
Added a basic exporter script along with a template prefab to generate another prefab currently according to the model's name etc.


          Fixed bugs and added an experimental prefab export operator

01082bd

flo suggested changes

View reviewed changes

Contributor

flo left a comment

Thanks for making a proper PR. Please have a look at my comments

blender_addons/io_md5_exporter/io_export_md5.py Outdated

		@@ -664,6 +665,11 @@ def createSubMesh(blenderFacesWithRelativeVertexIndices, blenderMesh, blenderMes

		Face(md5SubMesh, md5VerticesOfFace[0], md5VerticesOfFace[1], md5VerticesOfFace[2])

		def checkPath(filePath):#python does not create directory just the file. This function added to create t eh directory

Contributor

flo Jul 12, 2017

In python methods get documented by adding a string as first statement in the method.

>>> def test(a):
...     """This is the documentation"""
...     return 5
... 
>>> test.__doc__
'This is the documentation'

If you use """ to specify the string you can also have a string contain line breaks.

Besides that I would suggest to use a better method name. checkPath sounds like a function that checks if a path exists and does nothing otherwise. I would suggest to name the method createDirectoryForFileIfMissing

Also dir is a build in python method and thus a different variable name should be used. The method dir lists what attributes an object has. So it is a very helpful method for figuring out what attributes and methods an object has

blender_addons/io_md5_exporter/io_export_md5.py Outdated

@@ @@ -801,26 +807,35 @@ def treat_bone(b, parent = None): @@
                       for mesh in range (1, len(meshes)):
                         for submesh in meshes[mesh].submeshes:
                           submesh.bindtomesh(meshes[0])
+                    checkPath(modelFilePath)#python does not create directory just the file. This function added to create t eh directory

Contributor

flo Jul 12, 2017

Comment should not be necesary once the method has a proper name. Also there is a typo in it that you could fix in the methods documentation.

blender_addons/io_md5_exporter/io_export_md5.py Outdated

-                      errmsg = "IOError " #%s: %s" % (errno, strerror)
+                    except Exception as e:
+                      print(str(e))
+                      # errmsg = "IOError " #%s: %s" % (errno, strerror)

Contributor

flo Jul 12, 2017

Commented out code looks like debug code. Your change is an improvement as errmsg does not seem to get used at all so the exception basically just got swallowed.

To me it seems like it might be best to just not catch the exception.

blender_addons/io_md5_exporter/io_export_md5.py Outdated

                     buffer = buffer + meshes[0].to_md5mesh()
                     file.write(buffer)
                     file.close()
                     print( "saved mesh to " + modelFilePath )
-                if animationFilePath:
+                if animationFilePath:

Contributor

flo Jul 12, 2017

Added whitespace at the end, probably not intentional

blender_addons/io_md5_exporter/io_export_md5.py Outdated

                     #save animation file
+                    if modelFilePath == None:
+                      if len(meshes)>1:

Contributor

flo Jul 12, 2017

maybe add spaces around >

blender_addons/io_md5_exporter/io_export_md5.py Outdated

+                  with open(file_in_path, 'r') as inFile:
+                    data = json.load(inFile, object_pairs_hook=OrderedDict)
+                  ModelName=getSuggestedModelName()

Contributor

flo Jul 12, 2017

At least in java it is convention to have methods name start lower case and classes upper case. Since it is an exporter for a java program it might be a good idea to follow that convention. Especially not using upper case for a variable

blender_addons/io_md5_exporter/io_export_md5.py Outdated

+                    flag=1
+                    return anim
+                return null

Contributor

flo Jul 12, 2017

That looks wrong to me. In python there is no null but only None

Contributor Author

kartikey0303 Jul 12, 2017 •

edited

Yeah sorry didn't realized this. Just added this as a prototype function for later use

blender_addons/io_md5_exporter/io_export_md5.py Outdated

+                  data["skeletalmesh"]["material"]=ModelName+"Skin"
+                  data["skeletalmesh"]["animation"]=ModelName+"Idle"#later check if the animation actually exists
+                  data["WildAnimal"]["name"]=ModelName[:1].upper()+ModelName[1:]
+                  data["WildAnimal"]["icon"]="WildAnimals:"+ModelName+"Icon"

Contributor

flo Jul 12, 2017

spaces around = and + would make it more readable in my option like spaces in normal sentances make it easier to see word borders.

blender_addons/io_md5_exporter/io_export_md5.py Outdated

+                  data["WildAnimal"]["icon"]="WildAnimals:"+ModelName+"Icon"
+                  # data["Die"]["animationPool"]="["+findAnimation("death")+"]"
+                  assetDirectory=getTargetTerasologyAssetsDirectory(context)
+                  FileName=ModelName+".prefab"

Contributor

flo Jul 12, 2017

should start with lower case as it is not a type

blender_addons/io_md5_exporter/io_export_md5.py Outdated

+                      # value=context.scene.Scale
+                      # value=ExportMD5.md5scale.Scale
+                      # print("^^^^^^^^^^^",value)
+                      # col.prop(ExportMD5.md5scale,"Scale")

Contributor

flo Jul 12, 2017

commented out code: should be removed. Version history can keep track of old code versions.


          Made desired changes, added action list for testing

d56c25a

flo suggested changes

View reviewed changes

blender_addons/io_md5_exporter/io_export_md5.py Outdated

    
                    except IOError:

                      errmsg = "IOError " #%s: %s" % (errno, strerror)

                    except:

                      pass

Contributor

flo Jul 12, 2017

pass would mean simply ignoring the exception without any logging which would be bad

Contributor

flo Jul 12, 2017

I meant that the try and except get removed

blender_addons/io_md5_exporter/io_export_md5.py Outdated

                       Face(md5SubMesh, md5VerticesOfFace[0], md5VerticesOfFace[1], md5VerticesOfFace[2])
+              def createDirectoryForFileIfMissing(filePath):
+                """Function check if the directory is present, if not then creates one"""
+                directory=os.path.dirname(filePath)

Contributor

flo Jul 12, 2017

another case where spaces could be added around the =

blender_addons/io_md5_exporter/io_export_md5.py Outdated

@@ @@ -815,12 +822,19 @@ def treat_bone(b, parent = None): @@
                 if animationFilePath:
                     #save animation file
+                    if modelFilePath == None:
+                      if len(meshes)>1:

Contributor

flo Jul 12, 2017

another case where spaces could be added, please check for further cases by yourself

Contributor

flo commented Jul 12, 2017

I also tested it out and the export panel was not visible.

I had a look at the console and saw this error message:
found

Traceback (most recent call last):
  File "/home/flo/programs/blender-2.77a-linux-glibc211-x86_64/2.77/scripts/modules/addon_utils.py", line 330, in enable
    mod = __import__(module_name)
  File "/home/flo/.config/blender/2.77/scripts/addons/io_export_md5.py", line 1052, in <module>
    class actionlist_UL(UIList):
NameError: name 'UIList' is not defined

Contributor

flo commented Jul 12, 2017

Have a look at:
https://docs.blender.org/api/blender_python_api_2_70_5/bpy.types.UIList.html

I think for defined operators/lists etc. it might be necessary to follow blenders naming convention for them. It is a bit C like style. See that above docu also for an example.

kartikey0303 added 2 commits

July 14, 2017 02:58


          Bug fixes for action list

ac52f3c


          Added zip file export, multiple animation export still needs to be te…

f33e114

…sted

Member

Cervator commented Jul 18, 2017

Bump - there are new commits. @flo @Quaternius @nihal111 might one or more of you have a moment to please try out the latest version and see if it works? :-)

Cervator requested a review from Quaternius

July 18, 2017 04:20

kartikey0303 added 2 commits

July 18, 2017 09:55


          Bug fix - accidental rename

aae0b3a


          Added tab which was causing the bug

bc9984e

flo suggested changes

View reviewed changes

Contributor

flo left a comment

I tested it out and found the following issues:

The exported prefab is not usable ingame. I did some anlysis for you and found out that you export to the wrong directory (prefab instead of prefabs).
Also I noticed that the exported prefab still contains animationPool fields that reference the deer animations. Please change them as well to just contain the exported animation.
The prefab does not contain currently btw even in the animation field the proper animation. I exported the skeleton and the referenced animation was skeletonIdle but there was no such animation. I would have expected the selected animation to be chosen as animation.
The zip export resutls for me in a zip that contains also completly unrelated prefabas, models and animations that were just in the same directory. Files that are needed to make it work as module were completly missing. I would propose you remove that feature from this PR and make a new branch in which you add that feature. Then when this PR is merged you can make again a PR for the zip export feature.

Please test also next time that the exported stuff also works ingame.


          Some small changes

bca5330

Cervator merged commit bca5330 into MovingBlocks:master

Cervator added a commit that referenced this pull request


          Merge PR #49 by @kartikey0303 - Blender addon update

6e67736

Member

Cervator commented Jul 25, 2017

Tested locally - addon works! There are several outstanding issues but I'm documenting those with cards on the GitHub project page and merging this so we can start fresh with the remainder :-)

Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment