COLLADA export selecetd

Add option to only export selected elements.

Extracted from http://codereview.appspot.com/4530081/

[official.address243, 2011-06-19 20:59:55]

On 2011/06/19 20:13:21, pjoe wrote:

I admit I didn't test it...
But I read carefully through every line of the diff file and everything seems to
be fine.

[pjoe, 2011-06-20 06:12:11]

I'm actually not too fond of parsing export settings around as individual
parameters, but I guess that can be dealt with when we add more options
(modifiers, tangents, triangulate, etc.)

Would also be good to have a common way of dealing with export settings for all
C++ based exporters. For example I noticed the fbx exporter doesn't remember
it's settings.

[jesterKing, 2011-06-20 07:45:14]

On 2011/06/20 06:12:11, pjoe wrote:
> I'm actually not too fond of parsing export settings around as individual
> parameters, but I guess that can be dealt with when we add more options
> (modifiers, tangents, triangulate, etc.)
> 
> Would also be good to have a common way of dealing with export settings for
all
> C++ based exporters. For example I noticed the fbx exporter doesn't remember
> it's settings.

This exporter code is the first to introduce a settings file. And yes, it would
be nice to have it made generally available at some point.

[official.address243, 2011-06-20 14:11:00]

I would never deny that the storing and loading of the exporter settings are
"hacky", but they work AFAIK flawlessly - and that's what counts. And they is no
sensible alternative in Blender that I know of.
And yes: This feature is absolutely important. Applications which constantly
forget all their settings belong to the most annoying things in the computer
world I know of. Even if I write tiny applications I add storing and loading
settings, because it's so annoying and time consuming to apply the same settings
again and again - and if you do that sisyphus work you often enough forget a
setting - and you can start the whole process again! That eats far to much time
which could spent otherwise productively.

[pjoe, 2011-06-20 19:18:27]

On 2011/06/20 14:11:00, official.address243 wrote:
> I would never deny that the storing and loading of the exporter settings are
> "hacky", but they work AFAIK flawlessly - and that's what counts. And they is
no
> sensible alternative in Blender that I know of.

AFAIK it works great :) My comment wasn't meant to stop this patch from going
in, it was more of a forward looking thought and I guess I was a bit surprised
to see that there was no common way of handling this.

[dBugSlayer, 2011-06-22 07:01:59]

I tested the patch against r37607 exporting/import a few different simple scenes
and everything seemed fine. The tooltip when hovering the "Export only selected"
checkbox displays the following : "Python: WM_OT_collada_export.selected'. Is
this correct? Shouldn't it be somewhere in bpy.*?

Cheers,

Daniel

http://codereview.appspot.com/4636051/diff/1/source/blender/collada/DocumentE...
File source/blender/collada/DocumentExporter.cpp (right):

http://codereview.appspot.com/4636051/diff/1/source/blender/collada/DocumentE...
source/blender/collada/DocumentExporter.cpp:205: case OB_EMPTY:
This line's indentation has mixed tabs and spaces. I'm not sure how strict you
guys are about this sort of stuff, but I figured I'd mention anyways.

http://codereview.appspot.com/4636051/diff/1/source/blender/windowmanager/int...
File source/blender/windowmanager/intern/wm_files.c (right):

http://codereview.appspot.com/4636051/diff/1/source/blender/windowmanager/int...
source/blender/windowmanager/intern/wm_files.c:912: LinkNode *l, *lines;
This hunk has a bunch of trailing white spaces.

http://codereview.appspot.com/4636051/diff/1/source/blender/windowmanager/int...
File source/blender/windowmanager/intern/wm_operators.c (right):

http://codereview.appspot.com/4636051/diff/1/source/blender/windowmanager/int...
source/blender/windowmanager/intern/wm_operators.c:2015: MEM_freeN(filepath); 
// <-- Correct?
Can anyone chime-in on this comment?

http://codereview.appspot.com/4636051/diff/1/source/blender/windowmanager/wm_...
File source/blender/windowmanager/wm_files.h (right):

http://codereview.appspot.com/4636051/diff/1/source/blender/windowmanager/wm_...
source/blender/windowmanager/wm_files.h:41: #endif
Spotted another trailing white space

[pjoe, 2011-06-22 09:05:50]

Doh, I forgot to pay attention to white spaces when extracting this from the old
patch :(

I'll try to get around to fix this.

[pjoe, 2011-06-22 09:30:49]

Fixed white spaces

[jesterKing, 2011-06-22 11:42:14]

http://codereview.appspot.com/4636051/diff/3004/source/blender/collada/Docume...
File source/blender/collada/DocumentExporter.cpp (right):

http://codereview.appspot.com/4636051/diff/3004/source/blender/collada/Docume...
source/blender/collada/DocumentExporter.cpp:204: }
Any particular reason OB_EMPTY isn't included in the export_selected party?

[jesterKing, 2011-06-22 12:13:01]

http://codereview.appspot.com/4636051/diff/3004/source/blender/windowmanager/...
File source/blender/windowmanager/intern/wm_files.c (right):

http://codereview.appspot.com/4636051/diff/3004/source/blender/windowmanager/...
source/blender/windowmanager/intern/wm_files.c:908: #ifdef WITH_COLLADA
See comment on this where reading is used.

http://codereview.appspot.com/4636051/diff/3004/source/blender/windowmanager/...
File source/blender/windowmanager/intern/wm_operators.c (right):

http://codereview.appspot.com/4636051/diff/3004/source/blender/windowmanager/...
source/blender/windowmanager/intern/wm_operators.c:2011:
WM_read_collada_settings(&filepath, &selected);
After having read the code several times, I'm not too happy with the current
implementation. I'd like to see a more generic way for this right from the
beginning. Hence the configuration file would be better as a separate patch, to
keep this particular feature clean.

[official.address243, 2011-06-22 12:54:26]

Am 22.06.2011 13:42, schrieb nathan.letwory@gmail.com:
>
http://codereview.appspot.com/4636051/diff/3004/source/blender/collada/Docume...

>
> source/blender/collada/DocumentExporter.cpp:204: }
> Any particular reason OB_EMPTY isn't included in the export_selected
> party?
>
Well, hmmm.... I think that is actually wrong. You are right. It should 
be included. My thoughts about the role of OB_EMPTY were wrong.
Please, add it to the the list of objects handled by the export selection.

To the role of reading and writing the settings, as I said before: Yes 
of course this solution is hacky. But we must use that, otherwise we can 
wait another 6-12 months until someone finally comes up with a general 
solution to process that.
So I suggest: Implement that now, and when we have in the far future a 
general working patch, we use that instead.

Thanks, Jan

[pjoe, 2011-06-22 19:12:30]

Removed settings file after discussion with jesterKing

[official.address243, 2011-06-22 19:24:06]

On 2011/06/22 19:12:30, pjoe wrote:
> Removed settings file after discussion with jesterKing

That's sad, especially since I'm estimating that a general solution for this
lies far far in the future.

[pjoe, 2011-06-22 19:30:24]

On 2011/06/22 19:24:06, official.address243 wrote:
> That's sad, especially since I'm estimating that a general solution for this
> lies far far in the future.

I think keeping settings is really important (it's annoying as h*** when it
always goes back to default), but I tend to agree with jesterKing that this is a
separate issue from the 'export selected' functionality.

Anyway instead of arguing, let's get started on a 'remember export settings'
patch right away, and quickly find a good way of doing that.

[dBugSlayer, 2011-07-03 04:36:29]

How far are we from getting this patch applied? Do we need to submit it to the
issue tracker once we're done reviewing it?

Cheers,
Daniel

http://codereview.appspot.com/4636051/diff/15002/source/blender/windowmanager...
File source/blender/windowmanager/intern/wm_operators.c (right):

http://codereview.appspot.com/4636051/diff/15002/source/blender/windowmanager...
source/blender/windowmanager/intern/wm_operators.c:2007: char *filepath = 0;
These lines are no longer needed since the settings file is not being read
anymore.

[jesterKing, 2011-07-04 07:56:40]

On 2011/07/03 04:36:29, dBugSlayer wrote:
> How far are we from getting this patch applied? Do we need to submit it to the
> issue tracker once we're done reviewing it?

No need to send separately a patch. I first intended to apply the patch at the
end of previous week, but blender 2.58a schedule put it on hold. Since that's
now done I'll apply the patch asap.


> 
> Cheers,
> Daniel
> 
>
http://codereview.appspot.com/4636051/diff/15002/source/blender/windowmanager...
> File source/blender/windowmanager/intern/wm_operators.c (right):
> 
>
http://codereview.appspot.com/4636051/diff/15002/source/blender/windowmanager...
> source/blender/windowmanager/intern/wm_operators.c:2007: char *filepath = 0;
> These lines are no longer needed since the settings file is not being read
> anymore.

[jesterKing, 2011-07-04 09:00:06]

OK, this patch has now been committed in r38079

[jesterKing, 2011-07-04 09:03:39]

On 2011/07/04 09:00:06, jesterKing wrote:
> OK, this patch has now been committed in r38079

http://lists.blender.org/pipermail/bf-blender-cvs/2011-July/037282.html

[official.address243, 2011-07-04 13:42:47]

> OK, this patch has now been committed in r38079

Ah! Thanks :)

[dBugSlayer, 2011-07-04 19:59:13]

Sweet! Thanks Nathan.

On Jul 4, 2011, at 6:42, "official.address243@googlemail.com"
<official.address243@googlemail.com> wrote:

>> OK, this patch has now been committed in r38079
>
> Ah! Thanks :)
>
> http://codereview.appspot.com/4636051/