For naming, maybe this makes it more clear what it is. We also don't usually but the 3 letter prefix in the struct names.

BloWriter - > BlendWriter
BloReader -> BlendReader

Then for functions in blenkernel and modifiers, perhaps we can go along with the existing copy_data and free_data naming?

BKE_*_blo_write -> BKE_*_write_data
BKE_*_blo_read -> BKE_*_read_data

bloWrite -> writeData
bloRead -> readData

Out of curiosity: why are the write functions functions, but the read ones macro's?

Renaming to BlendWriter and BlendReader sounds good to me.

Then for functions in blenkernel and modifiers, perhaps we can go along with the existing copy_data and free_data naming?

I wanted to use a pattern that is more searchable than write_data and read_data. Also, names like these were very confusing in the context of point caches (and possibly other types) that have different read/write functions as well.

Out of curiosity: why are the write functions functions, but the read ones macro's?

The primary reason is that the read functions change data in-place. They could be functions well if that is preferred. I liked the way it looks when I use the api.

Ah yes, you like "the way it looks", that is a fair point, but does it outweigh the following?

• You can't debug a macro
• Error/warnings that the compiler generates in them generally make no sense
• Looks like a function call but isn't. So there may be hidden side effects, ie: I would not expect something that looks like a function call to modify a local variable without it being passed by reference, yet here we are.
• Complete lack of type information in most ide's (like you would get with properly typed functions)
• increased cost of maintenance of this piece of code due all the issues above.

Macro's are absolute code rot, and the bar to introduce new ones should be incredibly high, imho "I like the way it looks" does not reach that bar, not even close.

I'll change it to functions tomorrow.

Lets wait for one of the other devs weight in on this, for the last couple of years i seem to be alone in my anti macro stance, just because i have a strong opinion, doesn't mean i'm right or get to decide what happens in the blender codebase.

In D7061#168445, @Jacques Lucke (JacquesLucke) wrote:

I think it would be good to have a naming convention that has blo or blend or something similar in the name.

I think blend_read and blend_write could work well.

Should function names like BLO_read_int32_array be extended to indicate that they modify the input?
The issue is solved a little bit now, because you have to pass pointers to pointers into the functions.

I think that & is enough.

@Campbell Barton (campbellbarton) suggested to use the suffix _and_step or _and_advance for the read functions.

I'm confused, where does it step or advance? I don't see anything that requires these operations to be done in a particular order?

If there was I guess something like BLO_write_next_array could work.

I was still in the process of working on this document, but Jacques found it in the wiki already :). I've started a broader discussion about this now:
https://devtalk.blender.org/t/extensible-architecture-proposal/12115

If core developers broadly agree on going in this direction, I can create a more concrete task and plans.

In D7061#168541, @Bastien Montagne (mont29) wrote:

Generally looks fine. But I have one main concern about this patch: right now, it only implements a very small subset of what would be needed for a full move of blend read/write from BLO to BKE relevant files. I understand this was added for a specific need, but am afraid we'll then end up with endless refactor-that-is-never-finished, i.e. with some read/write code spread all over BKE, and most of it still in BLO. I would really rather avoid that, this would be even worse than having everything in BLO as we do now. This is not a small cleanup/extension, it has some rather important consequences over the whole bled file read/write system.

I think that's a fair concern, and I would like this to become used everywhere.

There is a bit of a precedent in that modifiers already take care of their own lib query, lib linking and depsgraph relations. So adding this is seems like a natural extension.

So question here is: Is there a proper plan to address this in coming months (let's say over the next six ones), is there a design task about it, and dedicated dev time to tackle it? What about moving ID read/write itself to new IDTypeInfo? What about lib_linking? What about library data linking/append?Would those changes affect the expand system too, or would that one remain properly confined to BLO area?

I think ideally all of that can be abstracted in APIs per datablock / modifier / node. To me modifiers seem like a good first step.

@Brecht Van Lommel (brecht), do you plan to create tasks for the different refactors mentioned in your wiki document?

I assume this patch is on hold until a broader plan is made?

In D7061#168463, @Brecht Van Lommel (brecht) wrote:

@Campbell Barton (campbellbarton) suggested to use the suffix _and_step or _and_advance for the read functions.

I'm confused, where does it step or advance? I don't see anything that requires these operations to be done in a particular order?

This macro steps the array forward, at the time of writing it was:

#define BLO_read_data_address(reader, ptr) \
  ptr = BLO_read_get_new_data_address(reader, ptr))

It's since been modified to take a pointer, to avoid hiding the assignment in a way that a function wouldn't be able to perform.

#define BLO_read_data_address(reader, ptr_p) \
  *(ptr_p) = BLO_read_get_new_data_address(reader, *(ptr_p))

However the naming still doesn't explicitly state it's advancing the pointer, which could be confusing since reading advances the pointer and writing doesn't.

It's replacing the old pointer written in the .blend with a new pointer that exists in RAM, not advancing it.

@Jacques Lucke (JacquesLucke), we can wait a week or so for feedback on the proposal, and then I can create more specific tasks. But I hope we can commit this soon after that.