Skip to content

Commit

Permalink
Fix missing serialization of ObjectPtr properties + copy property ins…
Browse files Browse the repository at this point in the history
…tead of property pointer to avoid crash in undo/redo of overriden Prefab properties
  • Loading branch information
Benualdo committed Oct 2, 2024
1 parent afa0de4 commit 401ac05
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 29 deletions.
54 changes: 54 additions & 0 deletions src/core/Object/Factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,28 @@ namespace vg::core
}
break;

case PropertyType::ObjectPtr:
{
IObject ** obj = _prop->GetPropertyObjectPtr(_object);

if (*obj)
{
auto uid = (*obj)->GetUID();
VG_VERIFY(_buffer.write(uid));

const char * className = (*obj)->GetClassName();
VG_VERIFY(_buffer.write((u32)strlen(className)));
VG_VERIFY(_buffer.write(className, strlen(className)));

SaveProperties(*obj, _bufferType);
}
else
{
VG_VERIFY(_buffer.write((UID)0x0));
}
}
break;

case PropertyType::ObjectPtrVector:
{
auto * vector = _prop->GetPropertyObjectPtrVector(_object);
Expand Down Expand Up @@ -1121,6 +1143,38 @@ namespace vg::core
}
break;

case PropertyType::ObjectPtr:
{
UID uid = 0;
VG_VERIFY(_buffer.restore(&uid, sizeof(UID), changed));

if (0 != uid)
{
u32 stringSize = 0;
char className[1024];
VG_ASSERT(stringSize < 1024);
VG_VERIFY(_buffer.restore(&stringSize, sizeof(u32), changed));
VG_VERIFY(_buffer.restore(className, stringSize, changed));
className[stringSize] = '\0';

auto ** obj = _prop->GetPropertyObjectPtr(_object);

if (nullptr != *obj)
{
RestoreProperties(*obj, _bufferType);
}
else
{
IObject * newObj = CreateObject(className, "", _object);
newObj->SetUID(uid);
RestoreProperties(newObj, _bufferType);
newObj->OnLoad();
*obj = newObj;
}
}
}
break;

case PropertyType::ObjectPtrVector:
{
u64 count = 0;
Expand Down
5 changes: 0 additions & 5 deletions src/core/Object/Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,7 @@ namespace vg::core
void Object::SetOriginalUID(UID _uid)
{
if (_uid != m_originalUID)
{
//IFactory * factory = Kernel::getFactory();
//if (m_originalUID)
// factory->ReleaseUID(m_originalUID);
m_originalUID = _uid;
}
}

//--------------------------------------------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion src/core/UndoRedo/Entry/UndoRedoPropertyEntry.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "core/IUndoRedo.h"
#include "core/File/Buffer.h"
#include "core/Object/Property.h"

namespace vg::core
{
Expand Down Expand Up @@ -31,7 +32,7 @@ namespace vg::core
io::Buffer m_original;
io::Buffer m_modified;

IProperty * m_prop;
Property m_prop;
IGameObject * m_prefab;
IDynamicProperty * m_propOverride;
};
Expand Down
27 changes: 13 additions & 14 deletions src/core/UndoRedo/Entry/UndoRedoPropertyEntry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ namespace vg::core
if (nullptr != _originalObject)
m_objectName = _originalObject->GetName();

m_prop = (IProperty *)_prop;
VG_ASSERT(m_prop);

VG_ASSERT(_prop);
m_prop = *((Property *)_prop);
m_prefab = _prefab;
m_propOverride = _propOverride;
}
Expand All @@ -24,14 +23,14 @@ namespace vg::core
void UndoRedoPropertyEntry::BeforeChange()
{
Factory * factory = (Factory *)Kernel::getFactory();
factory->serializePropertyToMemory(GetObject(), m_prop, m_original, BufferType::UndoRedo);
factory->serializePropertyToMemory(GetObject(), &m_prop, m_original, BufferType::UndoRedo);
}

//--------------------------------------------------------------------------------------
void UndoRedoPropertyEntry::AfterChange()
{
Factory * factory = (Factory *)Kernel::getFactory();
factory->serializePropertyToMemory(GetObject(), m_prop, m_modified, BufferType::UndoRedo);
factory->serializePropertyToMemory(GetObject(), &m_prop, m_modified, BufferType::UndoRedo);
}

//--------------------------------------------------------------------------------------
Expand All @@ -49,7 +48,7 @@ namespace vg::core
//--------------------------------------------------------------------------------------
string UndoRedoPropertyEntry::GetDescription() const
{
return fmt::sprintf("(%s)%s", asString(m_prop->GetType()), m_prop->GetDisplayName());
return fmt::sprintf("(%s)%s", asString(m_prop.GetType()), m_prop.GetDisplayName());
}

//--------------------------------------------------------------------------------------
Expand All @@ -58,11 +57,11 @@ namespace vg::core
if (auto * object = GetObject())
{
// Restore the original value serialized in memory before change
VG_INFO("[Undo/Redo] Undo Property \"%s\" (0x%016X) from Object \"%s\" (0x%016X)", m_prop->GetName(), m_prop, m_objectName.c_str(), object);
VG_INFO("[Undo/Redo] Undo Property \"%s\" (0x%016X) from Object \"%s\" (0x%016X)", m_prop.GetName(), m_prop, m_objectName.c_str(), object);

m_original.resetRead();
Factory * factory = (Factory *)Kernel::getFactory();
factory->serializePropertyFromMemory(object, m_prop, m_original, BufferType::UndoRedo);
factory->serializePropertyFromMemory(object, &m_prop, m_original, BufferType::UndoRedo);

if (m_prefab)
{
Expand All @@ -72,12 +71,12 @@ namespace vg::core
}
else
{
object->OnPropertyChanged(object, *m_prop);
object->OnPropertyChanged(object, m_prop);
}
}
else
{
VG_ERROR("[Undo/Redo] Cannot Undo Property \"%s\" (0x%016X) from Object \"%s\" (UID=0x%08X)", m_prop->GetName(), m_prop, m_objectName.c_str(), m_objectUID);
VG_ERROR("[Undo/Redo] Cannot Undo Property \"%s\" (0x%016X) from Object \"%s\" (UID=0x%08X)", m_prop.GetName(), m_prop, m_objectName.c_str(), m_objectUID);
}
}

Expand All @@ -87,11 +86,11 @@ namespace vg::core
if (auto * object = GetObject())
{
// Restore the modified value serialized in memory before change
VG_INFO("[Undo/Redo] Redo Property \"%s\" (0x%016X) from Object \"%s\" (0x%016X)", m_prop->GetName(), m_prop, m_objectName.c_str(), object);
VG_INFO("[Undo/Redo] Redo Property \"%s\" (0x%016X) from Object \"%s\" (0x%016X)", m_prop.GetName(), m_prop, m_objectName.c_str(), object);

m_modified.resetRead();
Factory * factory = (Factory *)Kernel::getFactory();
factory->serializePropertyFromMemory(object, m_prop, m_modified, BufferType::UndoRedo);
factory->serializePropertyFromMemory(object, &m_prop, m_modified, BufferType::UndoRedo);

if (m_prefab)
{
Expand All @@ -101,12 +100,12 @@ namespace vg::core
}
else
{
object->OnPropertyChanged(object, *m_prop);
object->OnPropertyChanged(object, m_prop);
}
}
else
{
VG_ERROR("[Undo/Redo] Cannot Redo Property \"%s\" (0x%016X) from Object \"%s\" (UID=0x%08X)", m_prop->GetName(), m_prop, m_objectName.c_str(), m_objectUID);
VG_ERROR("[Undo/Redo] Cannot Redo Property \"%s\" (0x%016X) from Object \"%s\" (UID=0x%08X)", m_prop.GetName(), &m_prop, m_objectName.c_str(), m_objectUID);
}
}
}
20 changes: 12 additions & 8 deletions src/engine/Component/Physics/Shape/PhysicsShapeComponent.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ namespace vg::engine
{
if (_parent)
{
if (m_shapeDesc == nullptr)
createShapeDesc();

if (m_shape == nullptr)
createShape();
// Because of the way Undo/Redo works, the shape desc should not be created here
//if (m_shapeDesc == nullptr)
// createShapeDesc();
//
//if (m_shape == nullptr)
// createShape();
}
}

Expand All @@ -55,7 +56,11 @@ namespace vg::engine
//--------------------------------------------------------------------------------------
void PhysicsShapeComponent::OnLoad()
{
createShape();
if (m_shapeDesc == nullptr)
createShapeDesc();

if (m_shape == nullptr)
createShape();

if (m_shapeDesc)
m_shapeDesc->RegisterUID();
Expand Down Expand Up @@ -157,13 +162,12 @@ namespace vg::engine
{
VG_SAFE_RELEASE(m_shape);
VG_ASSERT(m_shapeDesc);

if (m_shapeDesc)
m_shape = getPhysics()->CreateShape(m_shapeDesc);

if (auto * bodyComp = getBodyComponent())
{
m_shape->SetColor(bodyComp->getShapesColor());
}

return nullptr != m_shape;
}
Expand Down
12 changes: 11 additions & 1 deletion src/engine/GameObject/Prefab/PrefabGameObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ namespace vg::engine
//--------------------------------------------------------------------------------------
core::DynamicPropertyList * PrefabGameObject::getDynamicPropertyList(const core::IObject * _object) const
{
const auto uid = _object->GetOriginalUID();
const auto uid = _object->GetOriginalUID(false);
if (uid)
{
for (uint i = 0; i < m_dynamicProperties.size(); ++i)
Expand All @@ -99,6 +99,10 @@ namespace vg::engine
return propList;
}
}
else
{
VG_WARNING("[Prefab] Cannot get DynamicPropertyList for (%s)\"%s\" because it has no original UID", _object->GetClassName(), _object->GetName().c_str());
}
return nullptr;
}

Expand Down Expand Up @@ -139,6 +143,12 @@ namespace vg::engine
{
const auto flags = _prop->GetFlags();

if (!_object->GetUID(false))
{
VG_WARNING("[Prefab] Cannot override Property (%s)\"%s\" because Object \"%s\" has no UID", asString(_prop->GetType()).c_str(), _prop->GetName(), _object->GetName().c_str());
return false;
}

const bool isFolder = asBool(PropertyFlags::IsFolder & flags);
const bool isFile = asBool(PropertyFlags::IsFile & flags);

Expand Down

0 comments on commit 401ac05

Please sign in to comment.