char: fix crash during Character destructor (regression)

rdb · rdb · commit 3d49986ce266 · 2018-05-27T15:37:45.000+02:00
This was a regression in 0bb81a4 which was not keeping an edge case in mind if a weak pointer is locked during object destruction. This issue was fixed by 525a05ea2b1d00a1db901f883f1755943aeff33c, but that is not enough since it causes CharacterJointEffect::get_character() to return null now, so we need a separate method to check if a CharacterJointEffect belongs to a given character. This is more efficient, too. Fixes panda3d#330
diff --git a/panda/src/char/characterJoint.cxx b/panda/src/char/characterJoint.cxx
@@ -214,7 +214,7 @@ bool CharacterJoint::
 remove_net_transform(PandaNode *node) {
   CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());
   if (effect != (RenderEffect *)NULL &&
-      DCAST(CharacterJointEffect, effect)->get_character() == _character) {
+      DCAST(CharacterJointEffect, effect)->matches_character(_character)) {
     node->clear_effect(CharacterJointEffect::get_class_type());
   }
 
@@ -244,7 +244,7 @@ clear_net_transforms() {
 
     CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());
     if (effect != (RenderEffect *)NULL &&
-        DCAST(CharacterJointEffect, effect)->get_character() == _character) {
+        DCAST(CharacterJointEffect, effect)->matches_character(_character)) {
       node->clear_effect(CharacterJointEffect::get_class_type());
     }
   }
@@ -306,7 +306,7 @@ bool CharacterJoint::
 remove_local_transform(PandaNode *node) {
   CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());
   if (effect != (RenderEffect *)NULL &&
-      DCAST(CharacterJointEffect, effect)->get_character() == _character) {
+      DCAST(CharacterJointEffect, effect)->matches_character(_character)) {
     node->clear_effect(CharacterJointEffect::get_class_type());
   }
 
@@ -336,7 +336,7 @@ clear_local_transforms() {
 
     CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());
     if (effect != (RenderEffect *)NULL &&
-        DCAST(CharacterJointEffect, effect)->get_character() == _character) {
+        DCAST(CharacterJointEffect, effect)->matches_character(_character)) {
       node->clear_effect(CharacterJointEffect::get_class_type());
     }
   }
@@ -427,7 +427,7 @@ set_character(Character *character) {
 
         CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());
         if (effect != (RenderEffect *)NULL &&
-            DCAST(CharacterJointEffect, effect)->get_character() == _character) {
+            DCAST(CharacterJointEffect, effect)->matches_character(_character)) {
           node->clear_effect(CharacterJointEffect::get_class_type());
         }
       }
@@ -438,7 +438,7 @@ set_character(Character *character) {
 
         CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());
         if (effect != (RenderEffect *)NULL &&
-            DCAST(CharacterJointEffect, effect)->get_character() == _character) {
+            DCAST(CharacterJointEffect, effect)->matches_character(_character)) {
           node->clear_effect(CharacterJointEffect::get_class_type());
         }
       }
diff --git a/panda/src/char/characterJointEffect.I b/panda/src/char/characterJointEffect.I
@@ -27,3 +27,13 @@ INLINE PT(Character) CharacterJointEffect::
 get_character() const {
   return _character.lock();
 }
+
+/**
+ * Returns true if this CharacterJointEffect contains the given Character.
+ * This exists because it is faster to check than get_character() and can even
+ * be called while the Character is destructing.
+ */
+INLINE bool CharacterJointEffect::
+matches_character(Character *character) const {
+  return _character == character;
+}
diff --git a/panda/src/char/characterJointEffect.h b/panda/src/char/characterJointEffect.h
@@ -41,6 +41,8 @@ class EXPCL_PANDA_CHAR CharacterJointEffect : public RenderEffect {
   INLINE PT(Character) get_character() const;
 
 public:
+  INLINE bool matches_character(Character *character) const;
+
   virtual bool safe_to_transform() const;
   virtual bool safe_to_combine() const;
   virtual void output(ostream &out) const;

Original file line number	Diff line number	Diff line change
`@@ -214,7 +214,7 @@ bool CharacterJoint::`
`214`	`214`	`remove_net_transform(PandaNode *node) {`
`215`	`215`	`CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());`
`216`	`216`	`if (effect != (RenderEffect *)NULL &&`
`217`		`- DCAST(CharacterJointEffect, effect)->get_character() == _character) {`
	`217`	`+ DCAST(CharacterJointEffect, effect)->matches_character(_character)) {`
`218`	`218`	`node->clear_effect(CharacterJointEffect::get_class_type());`
`219`	`219`	`}`
`220`	`220`
`@@ -244,7 +244,7 @@ clear_net_transforms() {`
`244`	`244`
`245`	`245`	`CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());`
`246`	`246`	`if (effect != (RenderEffect *)NULL &&`
`247`		`- DCAST(CharacterJointEffect, effect)->get_character() == _character) {`
	`247`	`+ DCAST(CharacterJointEffect, effect)->matches_character(_character)) {`
`248`	`248`	`node->clear_effect(CharacterJointEffect::get_class_type());`
`249`	`249`	`}`
`250`	`250`	`}`
`@@ -306,7 +306,7 @@ bool CharacterJoint::`
`306`	`306`	`remove_local_transform(PandaNode *node) {`
`307`	`307`	`CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());`
`308`	`308`	`if (effect != (RenderEffect *)NULL &&`
`309`		`- DCAST(CharacterJointEffect, effect)->get_character() == _character) {`
	`309`	`+ DCAST(CharacterJointEffect, effect)->matches_character(_character)) {`
`310`	`310`	`node->clear_effect(CharacterJointEffect::get_class_type());`
`311`	`311`	`}`
`312`	`312`
`@@ -336,7 +336,7 @@ clear_local_transforms() {`
`336`	`336`
`337`	`337`	`CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());`
`338`	`338`	`if (effect != (RenderEffect *)NULL &&`
`339`		`- DCAST(CharacterJointEffect, effect)->get_character() == _character) {`
	`339`	`+ DCAST(CharacterJointEffect, effect)->matches_character(_character)) {`
`340`	`340`	`node->clear_effect(CharacterJointEffect::get_class_type());`
`341`	`341`	`}`
`342`	`342`	`}`
`@@ -427,7 +427,7 @@ set_character(Character *character) {`
`427`	`427`
`428`	`428`	`CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());`
`429`	`429`	`if (effect != (RenderEffect *)NULL &&`
`430`		`- DCAST(CharacterJointEffect, effect)->get_character() == _character) {`
	`430`	`+ DCAST(CharacterJointEffect, effect)->matches_character(_character)) {`
`431`	`431`	`node->clear_effect(CharacterJointEffect::get_class_type());`
`432`	`432`	`}`
`433`	`433`	`}`
`@@ -438,7 +438,7 @@ set_character(Character *character) {`
`438`	`438`
`439`	`439`	`CPT(RenderEffect) effect = node->get_effect(CharacterJointEffect::get_class_type());`
`440`	`440`	`if (effect != (RenderEffect *)NULL &&`
`441`		`- DCAST(CharacterJointEffect, effect)->get_character() == _character) {`
	`441`	`+ DCAST(CharacterJointEffect, effect)->matches_character(_character)) {`
`442`	`442`	`node->clear_effect(CharacterJointEffect::get_class_type());`
`443`	`443`	`}`
`444`	`444`	`}`