{"action":"created","issue":{"url":"https://api.github.com/repos/godotengine/godot-blender-exporter/issues/21","repository_url":"https://api.github.com/repos/godotengine/godot-blender-exporter","labels_url":"https://api.github.com/repos/godotengine/godot-blender-exporter/issues/21/labels{/name}","comments_url":"https://api.github.com/repos/godotengine/godot-blender-exporter/issues/21/comments","events_url":"https://api.github.com/repos/godotengine/godot-blender-exporter/issues/21/events","html_url":"https://github.com/godotengine/godot-blender-exporter/pull/21","id":321062118,"number":21,"title":"animation for transform","user":{"login":"Jason0214","id":19422835,"avatar_url":"https://avatars2.githubusercontent.com/u/19422835?v=4","gravatar_id":"","url":"https://api.github.com/users/Jason0214","html_url":"https://github.com/Jason0214","followers_url":"https://api.github.com/users/Jason0214/followers","following_url":"https://api.github.com/users/Jason0214/following{/other_user}","gists_url":"https://api.github.com/users/Jason0214/gists{/gist_id}","starred_url":"https://api.github.com/users/Jason0214/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Jason0214/subscriptions","organizations_url":"https://api.github.com/users/Jason0214/orgs","repos_url":"https://api.github.com/users/Jason0214/repos","events_url":"https://api.github.com/users/Jason0214/events{/privacy}","received_events_url":"https://api.github.com/users/Jason0214/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2018-05-08T06:47:08Z","updated_at":"2018-05-24T07:53:56Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/godotengine/godot-blender-exporter/pulls/21","html_url":"https://github.com/godotengine/godot-blender-exporter/pull/21","diff_url":"https://github.com/godotengine/godot-blender-exporter/pull/21.diff","patch_url":"https://github.com/godotengine/godot-blender-exporter/pull/21.patch"},"body":"### finish animation for node and skeleton transform\r\n    functional it works, but still have several details to discuss\r\n\r\n1. **the animation export format** \r\nin my implementation, if a node has animation data while there are no animation data in all its ancestors, then it will exported an AnimationPlayer as its sibling, besides all the animation data in its descendants all goes into that AnimationPlayer. I supposed it is what user wanted in most case.\r\n2. **the code structure** \r\nwith animation, now there are two traversal pass for the scene tree, the first pass is applied to Blender scene tree, dealing with skeleton, mesh and etc, also if there is animation data found in Blender node, then assign the Blender node to the corresponding Godot scene node's ```anim_data```. Then the second pass is applied to the Godot scene tree, dealing with all the found ```anim_data```. \r\nThere are two problems, first the ```anim_data``` (works as a temporary variable) is inappropriate attribute of```NodeTemplate```, may be I should inherit ```NodeTemplate```?  besides, the ```animation.py``` is not fall into the category of converters,\r\n3. **fix_matrix function**\r\n```fix_matrix```(used to convert a Blender matrix to Godot matrix) is a local function inside ```mat_to_string()```, however it need to be used in ```animation.py```, I am thinking about extract it into a global accessible position, but not sure where to put it\r\n4. **optimization in keyframe**\r\nsince Godot doesn't support Bezier interpolation, I exported animation by treat every frame as keyframe(maybe it's called baking?), it is definitely need to do some optimization to remove some not important keyframe, it is not implemented yet. I thought the collada importer of godot engine may have some code about it, but I could not find it (anyone give me a hint?)\r\n5. pylint warning for too many local variables, too many nested blocks, too many branches. I don't know how to solve it .. :P \r\n6. can we have a interface to add a internal_resource without providing a hashable object, it is hard to  have a hash for animations"},"comment":{"url":"https://api.github.com/repos/godotengine/godot-blender-exporter/issues/comments/391623361","html_url":"https://github.com/godotengine/godot-blender-exporter/pull/21#issuecomment-391623361","issue_url":"https://api.github.com/repos/godotengine/godot-blender-exporter/issues/21","id":391623361,"user":{"login":"sdfgeoff","id":13490050,"avatar_url":"https://avatars0.githubusercontent.com/u/13490050?v=4","gravatar_id":"","url":"https://api.github.com/users/sdfgeoff","html_url":"https://github.com/sdfgeoff","followers_url":"https://api.github.com/users/sdfgeoff/followers","following_url":"https://api.github.com/users/sdfgeoff/following{/other_user}","gists_url":"https://api.github.com/users/sdfgeoff/gists{/gist_id}","starred_url":"https://api.github.com/users/sdfgeoff/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdfgeoff/subscriptions","organizations_url":"https://api.github.com/users/sdfgeoff/orgs","repos_url":"https://api.github.com/users/sdfgeoff/repos","events_url":"https://api.github.com/users/sdfgeoff/events{/privacy}","received_events_url":"https://api.github.com/users/sdfgeoff/received_events","type":"User","site_admin":false},"created_at":"2018-05-24T07:53:56Z","updated_at":"2018-05-24T07:53:56Z","author_association":"COLLABORATOR","body":"IRC Conversation copied here for storage\r\n\r\n```\r\nsdfgeoff | So my only thought is that the function \"export_animation_track\" should perhaps be\r\n         | split up.\r\nsdfgeoff | The reliance on TRANSFORM_ATTRIBUTES also seems like it could be a bit better\r\n JasonLu | so if it is to split \"export_animation_track\", I think then extract the fcurves ->\r\n         | transform_frames_map out\r\n      <snip>\r\nsdfgeoff | Re: reliance: If the user has animated the color of an object, the energy of a lamp\r\n         | etc. etc. then using TRANSFORM_ATTRIBUTES here is a bit limiting\r\nsdfgeoff | If a developer in the future wants to add support for these they'll have to edit the\r\n         | export_animation_track function to allow each new type of animation.\r\n JasonLu | exactly\r\nsdfgeoff | I think that they shouldn't have to.\r\nsdfgeoff | Currently the export_animation_track is all about exporting the transformation tracks\r\n JasonLu | emmm, then we should have a separate function to convert fcurves to transform track\r\n JasonLu | and when others want to support animation about color, they add a new function\r\nsdfgeoff | I don't think they should have to add a new function. Surely a call to an existing\r\n         | function should be adequate.\r\nsdfgeoff | Something like create_track(node, fcurve, animation_name)\r\n JasonLu | the problem is that he value animation is quite different from transform animation, for\r\n         | example it does not need to convert to matrix and convert back\r\nsdfgeoff | create_track(node, fcurve, animation_name, function_to_run_on_every_keyframe_value) ?\r\n JasonLu | how about keeping the PR open, and I continue to finish the value animation,\r\n JasonLu | I first to do normally and  when complete we can discuss how to optimize it\r\nsdfgeoff | Yup, that sounds good to me\r\nsdfgeoff | My only thought is that it must be as easy as possible to add new animated properties -\r\n         | ideally without having to add or change the code in converters/animation.py\r\n JasonLu | emm, I will consider it,  I suppose it would be possible for different kinds of value\r\n         | transform (like color, shape key etc), but the transform track maybe can not be put\r\n         | into that category\r\n JasonLu | ** value animation track **\r\nsdfgeoff | In the end, I'm more concerned about functionality, so even if the transform track is a\r\n         | special case, that should be fine.\r\n```"}}
{"actor":{"display_login":"sdfgeoff"}}