Do we really need separate filter thing? It seems rather 1:1 mapping to ID codes here.

I'm not sure object copy should copy previews. Several areas creates temporary copies of object for which you don't really need previews and don't want time/memory to be wasted.

Why not to support terabytes. With alembic files around that'd make files list easier to read.

Not totally clear why do we need to duplicate path now actually Worth mentioning this somewhere.

Use fullstop insteaod of bangs in comments.

Not sure which parser you are using, but I can assure you argparse needs this, otherwise it 'eats'/ignores the first given arg...

Yes we need them, FILTER_ID_XX is bitflag... Mandatory to combine several ID types in a same filter (integer).

And we cannot do 1:1 mapping, because we have more than 32 IDs (see comment in DNA_ID.h). :/

Hmmm… Here I just followed what was done for existing IDs with previews (materials, textures, etc.).

I think we should keep same behavior regarding previews for all ID types. Also, copying previews might take a small amount of time, but generating previews (especially for objects, groups and scenes) is much more heavy process, so… not sure.

Hehe, this is just a raw oving of the function from below in code… But yes, good occasion to clean it up :P

well… it only applies to path, so… maybe renaming it 'use_path_free' then?

Path is now generated from root + relpath, no more static in filelist. Added some comment about it.

Use switch-case which will be compiled into a jump table. Not as if it's some bottleneck part of code, but still nice to avoid sloppyness when it's simple to do :)

Dead code is forbidden.

Do we really need that granularity level for filtering?

We had really similar discussion in the outliner filtering task, and the agreed way to go there was to use less granular filter flags (like Shading, Geometry, Light).

Its possible to use argparse in a way that its not needed. See: release/scripts/templates_py/background_job.py -> parser.parse_args(argv).

think in the context of the function its obvious that its only applying to the path.

Is there a good reason to use queue instead of BLI_task?

Its a tricky one - but think it could be worth not hard-coding in filter categories.

Asset browsers are going to have to their own kinds of categories anyway.

Couldn't there be a more generic tagging system? (so browsing blend files loads in one set of tags that can be swapped out for other kinds of asset browsers).

Tags (asset categories) can have icons, descriptions for tooltips.

Think it could be better not to include the filtering code in this patch. Then add some filtering/tagging system separately.

First, I’m not sure it is worth creating a task for each and every preview? Would mean potentially creating thousands of tasks in a very short amount of time…

But main reason is memory management. If we create one task per preview, then we have to pass FileListEntryCache *cache as pool userdata, and a single FileListEntryPreview *preview to each task's userdata - with free_taskdata set to true. But then, since preview will be systematically freed after filelist_cache_previewf is done, I’d need to duplicate it to add it to done queue…

So think using input and output queue, with limited number of working tasks, is more efficient here?

That whole func is dead anyway, not used anywhere. Just kept it (more or less) in sync with other changes I made, in case I need this zombie later…

I would not add tagging system to 'simple' filebrowser. Asset Engines are responsible of filtering and sorting items anyway, so they handle this themselves (Amber does have tag system in asset-engine branch, e.g.). And adding this to core filebrowser would mean we need to store those tags in .blend file, with a way to edit them, and then read them while listing datablocks in file, etc. Would really want to keep 'basic mode' as simple as possible.

However, a way to sort datablocks by type is mandatory if we want flat view, otherwise your fileview explode under brushes and other 'useless' IDs like that.

So if we really do not want a per-ID filtering, I’d rather follow Sergey's idea and define more 'global' filter types (we'd still need per-ID values internally anyway), something like that:

• Scenes
• Animations (actions)
• Objects & Groups
• Geometry (meshes, curves, lattice, armatures, metaballs)
• Shading (materials, nodetrees, textures, linestyles)
• Images, Movieclips & Sounds
• Environment (worlds, lamps, cameras, speakers)
• Misc (brushes, text, font, paintcurve, palette, mask, grease pencil)

Is it possible to reduce scope of the try-catch block?

Thing of objects is that they're being created/copied to a temporary storage during Cycles and Freestyle renderers. There you don't really want extra memory usage.

But even in the viewport, you can't reliably copy review. For example, you Shift-D an object which uses world space texture mapping.

But do we need the function? :)

Ok. Still would love to de-duplicate all the threading blocks we've got. Thinking of similar thing we did for parallel for, could do the same here. But wouldn't consider this is to be changed prior the merge.

It's a bit weird code. And more weird is why we don't need it anymore?

Picky, can use same indentation used elsewhere?

files = CollectionProperty(
        type=bpy.types.OperatorFileListElement,
        options={'HIDDEN', 'SKIP_SAVE'},
        )

also for other files...

odd name. call cmd ?

odd mixing append and list syntax...
would use

cmd.extend([
    ...
    ])

*picky* no need for parens.

input values can be const.

can call r_filelist

This does a full linked list count, suggest to add BLI_thread_queue_is_empty to master, and call that.

The thing is, a (python) crash in this part is *very* bad from user point of view, because it would leave unwanted crap in its file (temp scene etc.), and that part was rather brittle in early stage of dev. So this is more like a safety stuff to report the error but not fail on it.

Still, can split this big block into smaller ones, will do.

Oki, then we shall remove that copy from Groups as well I guess? Will do.

Good question, to which I do not have an answer yet. Thing is, we may need it again for asset engines (more precisely, to the 'create/edit assets' part, which is not fully defined yet). So… would rather keep it for now, easy to clean it up later in case we definitively do not need 'current' .blend file listing in filebrowser.

yes, see the point, but indeed not in scope of this patch. :)

That comes from the *thumbnail previewing* job, which has been removed now. Just remaining 'dirt', removed.