BaseObject::Free crashes my C4D [SOLVED]

Helper

On 25/06/2015 at 08:00, xxxxxxxx wrote:

Hi,

I have to note, the code I posted is not his code.
I might have mixed up some code from GVO and Message.
I've tested it now in GVO with a proper return (I was looking at some own code where I apply currentstatetoobject, but inside MSG_COMMAND (where it is allowed to insert objects), sorry for the confusion!) and it still crashes.

Also:

cd.doc = doc

This needs to be there, or the modelingcommand fails.
Either this: cd.doc = doc OR cd.doc = hh->GetDocument().

This is my code inside GVO (again, not from bamboudha, it may differ) (the concept should be the same) :

  
BaseObject* MyObjectData::GetVirtualObjects(BaseObject* op, HierarchyHelp* hh)  
{  
  BaseDocument* doc = hh->GetDocument();  
  BaseObject* cube = BaseObject::Alloc(Ocube);  
  if (!cube) return nullptr;  
  BaseObject* cubeRes;  
  PolygonObject* polyCube;  
  ModelingCommandData cd;  
  cd.doc = doc;  
  cd.op = cube;  
  if (!SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))  
  {  
      return nullptr;  
  }  
  else  
  {  
      cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));  
      if (!cubeRes) return nullptr;  
      polyCube = (PolygonObject* )cubeRes;  
      if (!polyCube) return nullptr;  
  }  
return polyCube  
}

If he has something like this, or if he maybe inserts this in a parent, and returns the parent, it's the same as my approach.
And I have the crash as well if I try to free my cube.
Feel free to test it yourself...

I was wondering that maybe the ModelingCommand would free it itself, but that makes no sense, I guess, because with MCOMMAND_CURRENTSTATETOOBJECT you have to free it.

Maybe some input from the moderators would be nice?

Greetings,
Casimir Smets

Helper

On 25/06/2015 at 08:16, xxxxxxxx wrote:

Typically, for GVO(), it is best to use a temporary 'fake' document. Simply use:

AutoAlloc<BaseDocument> tdoc;
if (!tdoc)
   return nullptr;

Use that in cd.doc. It might also be required that you insert the cube into this document for the command to work. Some don't require it, others do. When the method goes out of scope (you exit GVO()), both the BaseDocument and anything attached to it are automatically freed.

Helper

On 25/06/2015 at 08:44, xxxxxxxx wrote:

Hi Robert,

In my experience, you don't need to use a temporary fake document for MCOMMAND_MAKEEDITABLE or MCOMMAND_CURRENTSTATETOOBJECT. It works perfect with the active document.
And about inserting the cube, I've also tested this inside MSG_COMMAND, so my cube was allready created, and the same problem occured there.

It's just some weird problem if you try to free the object.

Greetings,
Casimir Smets

Helper

On 25/06/2015 at 15:17, xxxxxxxx wrote:

Here's some code which works correctly (obviously NOT called from GVO!) :

  
{   
     BaseObject *cube = BaseObject::Alloc(Ocube);   
     AutoAlloc<BaseDocument> doc2;   
     doc2->InsertObject(cube, nullptr, nullptr);   
  
     if(cube != nullptr)   
     {   
          ModelingCommandData cd;   
          cd.doc = doc2;   
          cd.op = cube;   
          if (SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))   
          {   
               BaseObject* polyCube = static_cast<BaseObject*>(cd.result->GetIndex(0));   
               if(polyCube != nullptr)   
               {   
                    doc->InsertObject(polyCube, nullptr, nullptr);   
                    doc->Message(MSG_UPDATE);   
               }   
          }   
     }   
}

It doesn't crash and there are no memory leaks because the temporary document goes out of scope and it and its objects are destroyed.

Steve

Helper

On 26/06/2015 at 00:45, xxxxxxxx wrote:

Hi guys,

Thanks for all the replies.
But I have to do this in GVO, otherwise the user can't change the points of the cube.

Best regards,
Bamboudha

Helper

On 30/06/2015 at 04:58, xxxxxxxx wrote:

Hi Bamboudha,

as Mohamed already asked in his first answer, please post us some code, so we can understand, what you are doing in GVO. I think, then we should be able to find a solution pretty fast.

Helper

On 30/06/2015 at 05:15, xxxxxxxx wrote:

Hi guys,

My code is practically the same as Casimir's.
Only difference is that I put my cube as a child of a baseobject and return that baseobject.

Originally posted by xxxxxxxx

 
BaseObject* MyObjectData::GetVirtualObjects(BaseObject* op, HierarchyHelp* hh)  
{  
  BaseDocument* doc = hh->GetDocument();  
  BaseObject* cube = BaseObject::Alloc(Ocube);  
  if (!cube) return nullptr;  
  BaseObject* cubeRes;  
  PolygonObject* polyCube;  
  ModelingCommandData cd;  
  cd.doc = doc;  
  cd.op = cube;  
  if (!SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))  
  {  
      return nullptr;  
  }  
  else  
  {  
      cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));  
      if (!cubeRes) return nullptr;  
      polyCube = (PolygonObject* )cubeRes;  
      if (!polyCube) return nullptr;  
  }  
return polyCube  
}

thanks for the help!
with kind regards,
Bamboudha

Helper

On 01/07/2015 at 05:50, xxxxxxxx wrote:

Hi,

I finally had the time to check. MCOMMAND_MAKEEDITABLE frees the original object internally. That's why you get the crashes. So you only need to free the cube in case of an error.
I admit, the docs are a bit vague in this point. I will add comments to the docs, but it may take some time, as I have to check all the other commands as well...

Furthermore I think, due to the temporary document Steve's code is a little bit cleaner. You simply don't have to worry about the command applying changes to the active document, which would not be allowed in GVO.

@Casimir: In your code in the end you double cast the result from the command.

        cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));
        if (!cubeRes) return nullptr;
        polyCube = (PolygonObject* )cubeRes;
        if (!polyCube) return nullptr;

There's no need for this. A cast is not a function executing any code. Here it's merely telling the compiler to interpret the memory the pointer is pointing to in a different way. So you tell the compiler: "Use the pointer as a pointer to BaseObject. Oh, wait a minute, forget it, use it as pointer to a PolygonObject."
And you also don't have to check for nullptr after each cast. The pointer doesn't change by getting casted. It's, as I already said, only being reinterpreted.
So this is enough:

        polyCube = static_cast<PolygonObject*>(cd.result->GetIndex(0));
        if (!polyCube) return nullptr;

I recommend some read on casting, for example on cplusplus.com.

Helper

On 01/07/2015 at 07:42, xxxxxxxx wrote:

Hi Andreas,

Thanks for your tips!
I will change them in my code and I will definately read some more on casting.

Greetings,
Casimir Smets

Helper

On 01/07/2015 at 07:43, xxxxxxxx wrote:

Hi,

Thank you very much for your explanation. very helpfull.

kind regards,
Bamboudha