refactor: standardize object type identification using uLibTypeMacro and update serialization macros
This commit is contained in:
@@ -19,3 +19,76 @@ The vtkHandlerWidget should handle the transformation of the puppet internal Con
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
## ACTIVATE PROPERTIES
|
||||
|
||||
ULIB_ACTIVATE_PROPERTIES must run after all member initialization, with the vtable pointing to the most-derived type. This is why it has to be in each constructor — in C++, virtual dispatch only works correctly after a class's vtable is installed, which happens at the start of each level's constructor body.
|
||||
|
||||
### Option 1 — End-of-class macro (no constructor boilerplate)
|
||||
Declare a private member activator as the last member of the class. Its constructor runs after all other members, and at that point the vtable is already Derived's:
|
||||
|
||||
|
||||
// In Property.h, add alongside ULIB_ACTIVATE_PROPERTIES:
|
||||
#define ULIB_DECLARE_PROPERTIES(SelfType) \
|
||||
private: \
|
||||
struct _PropActivator { \
|
||||
_PropActivator(SelfType* self) { \
|
||||
uLib::Archive::property_register_archive ar(self); \
|
||||
ar & *self; \
|
||||
} \
|
||||
} _prop_activator{this};
|
||||
Usage in ContainerBox.h — place it just before the closing brace:
|
||||
|
||||
|
||||
class ContainerBox : public TRS {
|
||||
public:
|
||||
// ... all constructors, no more ULIB_ACTIVATE_PROPERTIES(*this)
|
||||
|
||||
ULIB_DECLARE_PROPERTIES(ContainerBox) // ← replaces all 3 constructor calls
|
||||
};
|
||||
Tradeoff: Works perfectly for single-level classes. For hierarchies where multiple levels use the macro, RegisterDynamicProperty must deduplicate by name (skip if already registered). Requires one line per class in the class body, but zero lines in constructors.
|
||||
|
||||
### Option 2 — Lazy init via virtual InitProperties() in Object
|
||||
Modify Object to call a virtual hook on first GetProperties():
|
||||
|
||||
|
||||
// In Object.h:
|
||||
class Object {
|
||||
protected:
|
||||
virtual void InitProperties() {} // override in derived
|
||||
public:
|
||||
const std::vector<PropertyBase*>& GetProperties() const {
|
||||
if (!m_propertiesInitialized) {
|
||||
const_cast<Object*>(this)->m_propertiesInitialized = true;
|
||||
const_cast<Object*>(this)->InitProperties();
|
||||
}
|
||||
return m_properties;
|
||||
}
|
||||
};
|
||||
Then a CRTP base handles the rest without any macro:
|
||||
|
||||
|
||||
template<typename Derived>
|
||||
class PropertyObject : public Object {
|
||||
protected:
|
||||
void InitProperties() override {
|
||||
uLib::Archive::property_register_archive ar(this);
|
||||
ar & *static_cast<Derived*>(this);
|
||||
}
|
||||
};
|
||||
Usage — just change the base class:
|
||||
|
||||
|
||||
class ContainerBox : public PropertyObject<ContainerBox>, public TRS { ... };
|
||||
// Nothing else needed — properties activated on first GetProperties() call
|
||||
Tradeoff: Most "automatic" — pure inheritance, no constructor or class-body macros. But requires modifying Object (adding m_propertiesInitialized flag + virtual hook), and lazy init means properties aren't available until first access. Also doesn't work well with multiple inheritance (which TRS likely involves).
|
||||
|
||||
Option 3 — CRTP doesn't work from the base constructor
|
||||
Just to be explicit: a CRTP base that calls ULIB_ACTIVATE_PROPERTIES in its own constructor won't work, because when PropertyObject<Derived>'s constructor runs, the vtable is PropertyObject<Derived>'s — Derived::serialize() hasn't been installed yet. So ar & *self calls Object::serialize() (a no-op).
|
||||
|
||||
Recommendation
|
||||
Option 1 is the least invasive and safest. Add deduplication to RegisterDynamicProperty in Object.cpp to guard against re-registration when hierarchies stack activators, then replace every ULIB_ACTIVATE_PROPERTIES(*this) in constructors with a single ULIB_DECLARE_PROPERTIES(ClassName) at the end of the class body.
|
||||
|
||||
Option 2 is cleaner to use but requires changing the Object interface and has the lazy-init semantic change — only worth it if you want zero-touch activation across the entire framework.
|
||||
Reference in New Issue
Block a user