fix(joints): snap crashing datamodel

This commit is contained in:
maelstrom 2025-04-20 18:50:27 +02:00
parent e9757ab306
commit 722c4acfb4
10 changed files with 73 additions and 130 deletions

View file

@ -41,6 +41,7 @@ struct InstanceType {
typedef std::pair<std::shared_ptr<Instance>, std::string> _RefStatePropertyCell;
class DescendantsIterator;
class Snap;
// Base class for all instances in the data model
// Note: enable_shared_from_this HAS to be public or else its field will not be populated
@ -58,6 +59,8 @@ private:
bool ancestryContinuityCheck(std::optional<std::shared_ptr<Instance>> newParent);
void updateAncestry(std::optional<std::shared_ptr<Instance>> child, std::optional<std::shared_ptr<Instance>> newParent);
friend Snap; // This isn't ideal, but oh well
protected:
bool parentLocked = false;
std::unique_ptr<MemberMap> memberMap;

View file

@ -193,83 +193,17 @@ std::shared_ptr<DataModel> DataModel::CloneModel() {
// Clone services
for (std::shared_ptr<Instance> child : GetChildren()) {
// Special case: Instances parented to DataModel which are not services
if (!(child->GetClass()->flags & INSTANCE_SERVICE)) {
auto result = child->Clone(state);
if (result)
newModel->AddChild(result.value());
auto result = child->Clone(state);
if (!result)
continue;
}
DataModel::cloneService(newModel, std::dynamic_pointer_cast<Service>(child), state);
}
// Clone children of services
for (const auto& [className, service] : services) {
for (auto child : service->GetChildren()) {
auto result = child->Clone(state);
if (result)
service->AddChild(result.value());
newModel->AddChild(result.value());
// Special case: Ignore instances parented to DataModel which are not services
if (child->GetClass()->flags & INSTANCE_SERVICE) {
newModel->services[child->GetClass()->className] = std::dynamic_pointer_cast<Service>(result.value());
}
}
return newModel;
}
void DataModel::cloneService(std::shared_ptr<DataModel> target, std::shared_ptr<Service> service, RefState<_RefStatePropertyCell> state) {
// !!!!! THIS MAY CAUSE PROBLEMS !!!!!
// The way we are updating references means that their property onUpdate handler
// gets called immediately. They may expect certain invariants to be held such as workspace existing, while this
// is not the case. Currently, since this invariant is only used in the case of referring directly to workspace etc.,
// this is fine, since GetService will avoid duplication problems. But here be dragons.
std::shared_ptr<Instance> newInstance = target->GetService(service->GetClass()->className).expect();
// Copy properties
for (std::string property : service->GetProperties()) {
PropertyMeta meta = service->GetPropertyMeta(property).expect();
if (meta.flags & (PROP_READONLY | PROP_NOSAVE)) continue;
// Update InstanceRef properties using map above
if (meta.type == &Data::InstanceRef::TYPE) {
std::weak_ptr<Instance> refWeak = service->GetPropertyValue(property).expect().get<Data::InstanceRef>();
if (refWeak.expired()) continue;
auto ref = refWeak.lock();
auto remappedRef = state->remappedInstances[ref]; // TODO: I think this is okay? Maybe?? Add null check?
if (remappedRef) {
// If the instance has already been remapped, set the new value
newInstance->SetPropertyValue(property, Data::InstanceRef(remappedRef)).expect();
} else {
// Otheriise, queue this property to be updated later, and keep its current value
auto& refs = state->refsAwaitingRemap[ref];
refs.push_back(std::make_pair(newInstance, property));
state->refsAwaitingRemap[ref] = refs;
newInstance->SetPropertyValue(property, Data::InstanceRef(ref)).expect();
}
} else {
Data::Variant value = service->GetPropertyValue(property).expect();
newInstance->SetPropertyValue(property, value).expect();
}
}
// Remap self
state->remappedInstances[service->shared_from_this()] = newInstance;
// Add service prior to adding children, as children may expect service to already be parented to DataModel
target->AddChild(newInstance);
target->services[service->GetClass()->className] = std::dynamic_pointer_cast<Service>(newInstance);
// Remap queued properties
for (std::pair<std::shared_ptr<Instance>, std::string> ref : state->refsAwaitingRemap[service->shared_from_this()]) {
ref.first->SetPropertyValue(ref.second, Data::InstanceRef(newInstance)).expect();
}
// Clone children
// for (std::shared_ptr<Instance> child : service->GetChildren()) {
// std::optional<std::shared_ptr<Instance>> clonedChild = child->Clone(state);
// if (clonedChild)
// newInstance->AddChild(clonedChild.value());
// }
}

View file

@ -53,17 +53,16 @@ public:
result<std::optional<std::shared_ptr<Service>>, NoSuchService> FindService(std::string className);
template <typename T>
result<std::shared_ptr<T>, NoSuchService> GetService() {
std::shared_ptr<T> GetService() {
auto result = GetService(T::TYPE.className);
if (result.isError()) return result.error().value();
return std::dynamic_pointer_cast<T>(result.success().value());
return std::dynamic_pointer_cast<T>(result.expect("GetService<T>() was called with a non-service instance type"));
}
template <typename T>
std::optional<std::shared_ptr<T>> FindService() {
auto result = FindService(T::TYPE.className);
if (result.isError()) return result.error().value();
return std::dynamic_pointer_cast<T>(result.success().value());
auto result = FindService(T::TYPE.className).expect("FindService<T>() was called with a non-service instance type");
if (!result) return std::nullopt;
return std::dynamic_pointer_cast<T>(result.value());
}
// Saving/loading

View file

@ -1,4 +1,5 @@
#include "jointsservice.h"
#include "workspace.h"
const InstanceType JointsService::TYPE = {
.super = &Instance::TYPE,
@ -21,3 +22,9 @@ void JointsService::InitService() {
if (initialized) return;
initialized = true;
}
std::optional<std::shared_ptr<Workspace>> JointsService::jointWorkspace() {
if (!dataModel()) return std::nullopt;
return dataModel().value()->FindService<Workspace>();
}

View file

@ -2,7 +2,12 @@
#include "objects/base/service.h"
class Snap;
class JointsService : public Service {
private:
std::optional<std::shared_ptr<Workspace>> jointWorkspace();
friend Snap;
protected:
void InitService() override;
bool initialized = false;

View file

@ -8,6 +8,7 @@
#include "part.h"
#include <memory>
#include <reactphysics3d/constraint/FixedJoint.h>
#include <reactphysics3d/engine/PhysicsWorld.h>
const InstanceType Snap::TYPE = {
.super = &Instance::TYPE,
@ -52,66 +53,45 @@ Snap::~Snap() {
}
void Snap::OnAncestryChanged(std::optional<std::shared_ptr<Instance>>, std::optional<std::shared_ptr<Instance>>) {
// If the old workspace existed, and the new one differs, delete the current joint
if (this->joint && !this->oldWorkspace.expired() && (!workspace() || workspace().value() != this->oldWorkspace.lock())) {
// printf("Broke joint - Removed from workspace\n");
oldJointWorkspace.lock()->physicsWorld->destroyJoint(this->joint);
this->joint = nullptr;
}
// Destroy and rebuild the joint, it's the simplest solution that actually works
// If the previous parent was JointsService, and now it isn't, delete the joint
if (this->joint && !oldParent.expired() && oldParent.lock()->GetClass() == &JointsService::TYPE && (!GetParent() || GetParent() != oldParent.lock())) {
// printf("Broke joint - Removed from JointsService\n");
oldJointWorkspace.lock()->physicsWorld->destroyJoint(this->joint);
this->joint = nullptr;
}
// If the new workspace exists, and the old one differs, create the joint
if (!this->joint && workspace() && (oldWorkspace.expired() || oldWorkspace.lock() != workspace().value())) {
// printf("Made joint - Added to workspace\n");
buildJoint();
}
// If the new parent is JointsService and the previous wasn't, then create the joint
if (!this->joint && GetParent() && GetParent().value()->GetClass() == &JointsService::TYPE && (oldParent.expired() || GetParent() != oldParent.lock())) {
// printf("Made joint - Added to JointsService\n");
buildJoint();
}
this->oldParent = !GetParent() ? std::weak_ptr<Instance>() : GetParent().value();
this->oldWorkspace = !workspace() ? std::weak_ptr<Workspace>() : workspace().value();
this->oldJointWorkspace = !jointWorkspace() ? std::weak_ptr<Workspace>() : jointWorkspace().value();
breakJoint();
buildJoint();
}
void Snap::onUpdated(std::string property) {
// We are not in the workspace, so we don't really care what values are currently set
if (!jointWorkspace()) return;
// Workspace cannot have changed, so if the joint currently exists, it is in the present one
if (this->joint)
jointWorkspace().value()->physicsWorld->destroyJoint(this->joint);
// Destroy and rebuild the joint, if applicable
breakJoint();
buildJoint();
}
void Snap::buildJoint() {
if (part0.expired() || part1.expired() || !jointWorkspace()) return;
// Only if both parts are set, are not the same part, are part of a workspace, and are part of the same workspace, we build the joint
if (part0.expired() || part1.expired() || part0.lock() == part1.lock() || !part0.lock()->workspace() || part0.lock()->workspace() != part1.lock()->workspace()) return;
// Don't build the joint if we're not part of either a workspace or JointsService
if ((!GetParent() || GetParent().value()->GetClass() != &JointsService::TYPE) && !workspace()) return;
std::shared_ptr<Workspace> workspace = part0.lock()->workspace().value();
if (!workspace->physicsWorld) return;
// Update Part1's rotation and cframe prior to creating the joint as reactphysics3d locks rotation based on how it
// used to be rather than specifying an anchor rotation, so whatever.
Data::CFrame newFrame = part0.lock()->cframe * (c1.Inverse() * c0);
part1.lock()->cframe = newFrame;
jointWorkspace().value()->SyncPartPhysics(part1.lock());
workspace->SyncPartPhysics(part1.lock());
rp::FixedJointInfo jointInfo(part0.lock()->rigidBody, part1.lock()->rigidBody, (c0.Inverse() * c1).Position());
this->joint = dynamic_cast<rp::FixedJoint*>(jointWorkspace().value()->physicsWorld->createJoint(jointInfo));
this->joint = dynamic_cast<rp::FixedJoint*>(workspace->physicsWorld->createJoint(jointInfo));
jointWorkspace = workspace;
}
std::optional<std::shared_ptr<Workspace>> Snap::jointWorkspace() {
if (workspace()) return workspace();
// !!! REMINDER: This has to be called manually when parts are destroyed/removed from the workspace, or joints will linger
void Snap::breakJoint() {
// If the joint doesn't exist, or its workspace expired (not our problem anymore), then no need to do anything
if (!this->joint || jointWorkspace.expired() || !jointWorkspace.lock()->physicsWorld) return;
if (GetParent() && GetParent().value()->GetClass() == &JointsService::TYPE)
return std::dynamic_pointer_cast<DataModel>(GetParent().value()->GetParent().value())->GetService<Workspace>();
return {};
jointWorkspace.lock()->physicsWorld->destroyJoint(this->joint);
this->joint = nullptr;
}

View file

@ -10,15 +10,11 @@ class Workspace;
class Snap : public Instance {
rp::FixedJoint* joint = nullptr;
std::weak_ptr<Instance> oldParent;
// The actual workspace the joint is a part of
std::weak_ptr<Workspace> oldWorkspace;
// The pseudo-workspace the joint is a part of (including if parented to JointsService)
std::weak_ptr<Workspace> oldJointWorkspace;
// The workspace the joint was created in, if it exists
std::weak_ptr<Workspace> jointWorkspace;
protected:
void OnAncestryChanged(std::optional<std::shared_ptr<Instance>>, std::optional<std::shared_ptr<Instance>>) override;
std::optional<std::shared_ptr<Workspace>> jointWorkspace();
void onUpdated(std::string property);
void buildJoint();
void breakJoint();

View file

@ -1,5 +1,7 @@
#include "workspace.h"
#include "objects/base/instance.h"
#include "objects/jointsservice.h"
#include "objects/snap.h"
#include "physics/util.h"
#include <reactphysics3d/engine/PhysicsCommon.h>
@ -47,6 +49,20 @@ void Workspace::InitService() {
std::shared_ptr<Part> part = std::dynamic_pointer_cast<Part>(obj);
this->SyncPartPhysics(part);
}
// Activate all joints
for (auto it = this->GetDescendantsStart(); it != this->GetDescendantsEnd(); it++) {
InstanceRef obj = *it;
if (obj->GetClass()->className != "Snap") continue; // TODO: Replace this with a .IsA call instead of comparing the class name directly
std::shared_ptr<Snap> joint = std::dynamic_pointer_cast<Snap>(obj);
joint->UpdateProperty("Part0");
}
for (auto obj : dataModel().value()->GetService<JointsService>()->GetChildren()) {
if (obj->GetClass()->className != "Snap") continue; // TODO: Replace this with a .IsA call instead of comparing the class name directly
std::shared_ptr<Snap> joint = std::dynamic_pointer_cast<Snap>(obj);
joint->UpdateProperty("Part0");
}
}
void Workspace::SyncPartPhysics(std::shared_ptr<Part> part) {

View file

@ -177,7 +177,7 @@ MainWindow::MainWindow(QWidget *parent)
snap->c1 = part0->cframe;
// gWorkspace()->AddChild(snap);
gDataModel->GetService<JointsService>().expect()->AddChild(snap);
gDataModel->GetService<JointsService>()->AddChild(snap);
}
void MainWindow::closeEvent(QCloseEvent* evt) {

View file

@ -296,7 +296,7 @@ void PropertiesView::setSelected(std::optional<InstanceRef> instance) {
PropertyMeta meta = inst->GetPropertyMeta(property).expect();
Data::Variant currentValue = inst->GetPropertyValue(property).expect();
if (meta.type == &Data::CFrame::TYPE) continue;
// if (meta.type == &Data::CFrame::TYPE) continue;
QTreeWidgetItem* item = new QTreeWidgetItem;
item->setFlags(item->flags() | Qt::ItemIsEditable | Qt::ItemIsSelectable);
@ -311,6 +311,9 @@ void PropertiesView::setSelected(std::optional<InstanceRef> instance) {
} else if (meta.type == &Data::Vector3::TYPE) {
Data::Vector3 vector = currentValue.get<Data::Vector3>();
item->setData(1, Qt::DisplayRole, QString::fromStdString(currentValue.ToString()));
} else if (meta.type == &Data::CFrame::TYPE) {
Data::Vector3 vector = currentValue.get<Data::CFrame>().Position();
item->setData(1, Qt::DisplayRole, QString::fromStdString(currentValue.ToString()));
} else {
item->setData(1, Qt::DisplayRole, QString::fromStdString(currentValue.ToString()));
}