Standardize TypedWritable

[NWPlayer123]

I've been implementing BAM support recently and with that, I've hit a lot of flaws in documentation of how it actually works. There's putil/typedWritable.C.template and putil/typedWritable.h.template but they haven't been touched since Panda3D was originally open sourced, and in the first handful of commits, make_Generic shifted to make_from_bam, but a lot of objects were never updated, including the supposed templates.

You can see which ones use make_from_bam and which use the older style by searching for register_factory, which brings up a lot of objects that haven't been touched since the initial revision 24 years ago. There's also this documentation that doesn't even get the name of the template file right.

While I'm here, do we even need register_with_read_factory if we have to initialize it manually instead of using a vtable call? The majority of register_with_read_factory functions only have a single registration, there's only 3 I saw that need multiple.

If you combine all registrations into the config files, you only have to call BamReader::get_factory() once per config instead of once per registry function. Either way, you need to refer to individual classes, so you may as well just refer to their functions you're trying to register instead of having a one-liner function in 200+ different files.

I've just found it hard to track down where each object is reading data from the BamReader, you have to look at fillin but it's not always called when reading a BAM file so you have to look for make_from_bam but it's not always called that so you have to look at register_with_read_factory. I don't want three layers of abstraction for every object I look at, I want one.

[rdb]

Thank you for sharing your frustrations.

We can't use a vtable call for register_with_read_factory since those can only be done on an already-constructed object. I'm not sure what the Panda designers' reason was to create a separate method for this, it does seem slightly unnecessary. I'd be happy to consider a change to directly call register_factory in the config files.

The reason why we do need a separate virtual fillin method is that further updates to a particular object will end up needing to call fillin again in order to adopt the changed fields. This is useful when synchronizing Panda scenes over a socket connection.

For the default TextureStage, it is important that it is always resolved to the singleton default TextureStage, so checking the field and returning this pointer in make_from_bam was the easiest way to implement this. This isn't actually the best way to implement this, since it breaks when streaming changes to this object. It should instead just "peek" at the first value in the datagram, or it should implement change_this like RenderState does.

I wasn't aware of the existence of those templates or documentation files. I agree we need to update or remove them, and also update the remaining cases of factory functions that are not called make_from_bam.

Please note the existence of this document, which contains my own notes as I wrote my own bam writer library, though it looks like you might already be largely past the point where this might be useful.

[NWPlayer123]

We can't use a vtable call for register_with_read_factory since those can only be done on an already-constructed object. I'm not sure what the Panda designers' reason was to create a separate method for this, it does seem slightly unnecessary. I'd be happy to consider a change to directly call register_factory in the config files.

Ah, right. The only reason I could see keeping registration in each cxx is to keep all class-specific behavior there, but now that every registered function is called make_from_bam and is easy enough to find, it'd be better to centralize it in the config files.

The reason why we do need a separate virtual fillin method is that further updates to a particular object will end up needing to call fillin again in order to adopt the changed fields. This is useful when synchronizing Panda scenes over a socket connection.

Makes sense, I haven't looked into runtime behavior much yet.

Please note the existence of this document, which contains my own notes as I wrote my own bam writer library, though it looks like you might already be largely past the point where this might be useful.

Saves me the work of having to go through each bam revision when adding crate documentation, thanks!