This does not seem to be needed at all?

Aren't you missing to set history->browse_mode here?

Hrrrrrm...
BIP FLASH BIP
ACCESS AFTER FREE
;)

session UUID

id_session_uuid

Why?

Am a bit concerned about this storage of ID pointers, did you ensure this get cleared/rebuilt after any undo/redo? Not to mention plain basic file load (when not loading UI)?

Some more changes that should be committed separately as cleanups.

I'd rather have an explicit proper comparison of members of asset_repository that should match, in a util function. Assuming a memory comparison is OK here is weak, to say the least.

!= 0 is enough here, and is convention we use all across our codebase currently (except when checked value is compound bitflags).

Any reason not to call filelist_clear_ex(filelist, false, false, true); above instead?

filelist->asset_repository != NULL

MAIN_ID_SESSION_UUID_UNSET

filelist->id_map == NULL

Am confused, I though 'unset' value was -1 now?

Why can't you simply use FILE_TYPE_ASSET from eFileSel_File_Types instead?

!"Nice message explaining the assert reason." instead of just 0.

Same as above.

Can we keep ID for IDs in Blender? Could be UID here e.g.

same here, either just name, or uid_name?

Steel one from existing char _pad0[6] instead!

Thinks this needs an extra void *_pad1_v; or something? Remember safe way to deal with pointers in DNA is to always have an even of consecutive ones...

Why?

Why is this changed to 64bits?

id_session_uuid

We try to avoid magic values like that, please use rather a define like #define FILEDIR_NBR_ENTRIES_UNSET -1 e.g.

And then check against that define, instead of the >= 0 or == -1 above in code.

Looks like it, good catch! This worked very well in my tests so I'm surprised in hindsight.

Yeah, noticed and fixed that yesterday. I swear, check the commit date on the next patch update! :D

Quick code archeology... Ah, seems it's not needed anymore, I used to store the FILE_TYPE_ASSET as (1 << 32) but that changed.

No I didn't! But it's the main blocking TODO for the merge: T83242: Ensure "Current File" repository stays in sync with main data-base.

(Also, see my first comment here.)

It is a compound one actually: FILE_TYPE_ASSET | (1 << 29). But I don't like that and will change it.

Don't think so, looks like an oversight.

It's set to 0 in filelist_readjob_startjob(), but indeed, that's confusing. Should probably just set it to 0 here to declare it as a valid, empty list.

Hmm, but I stole one from _pad1. Isn't that better since _pad0 is part of the SpaceLink base?
Looking at write/read code, we write the data with the correct SDNA ID (ie not SpaceLink) so reconstruction should be fine, even if the _pad0 is used. But still looks like a potential source of problems.

Same as above I think, because I used 1 << 32 in the beginning which would cause warnings otherwise.

I'm not sure what you mean. That I don't just set FILE_TYPE_ASSET for the filter parameter? That wouldn't work since the filters are effectively OR'ed together, but for "Only Assets" we want to AND it to the other filter options, if that makes sense?

Or do you mean adding a new parameter taking the file types to limit visibility to?

Even better, removed the assert and the return. Since we cast to eFileBrowse_Mode above, the compiler is smart enough to see that this point can not be reached, and won't compile if a case is not handled. (Think usually if a enum case isn't handled you just get a warning?)

Hmm, may I disagree? :) For me name is something shown in the UI, but isn't necessarily unique. If it's a unique identifier -> idname. Grepping over the code - we use idname quite a lot, and it's obvious what it is. In the cases where we use just name, I'm not sure if this is an identifier or just a UI name. Didn't find any usages of uid_name.

We don't seem to use the _v suffix anywhere else, so going with void *_pad2.

Hrmm, seems GCC doesn't like that idea. Will have to bring the return back.

Ak, why is that still there? Removed locally now.

WTH... what was I doing there?

But file types passed via filter are OR'ed together. That's not what we want for FILE_ASSETS_ONLY, there we need a logical AND. It should remove all files that are *not* FILE_TYPE_ASSET, however filter keeps files that *are* of the given types.

So filter includes, filter_assets_only excludes files.
We could of course just make the filtering behave differently for that file type filter but, eeeh...

Well, the surprising thing is that Clang did *not* complain. This is clearly a possible source of bugs, as GCC correctly complains about.

We don't really need this, but most members of FileDirEntry are already accessed via filelist.c only. Think with just 2 or 3 more of these dumb accessors, we can move the struct out of DNA into filelist.c. I find that a good idea, but will do whatever you prefer.

Added a task to the workboard: T83656: Code cleanup