Code Review?

THE POST BELOW IS MORE THAN 5 YEARS OLD. RELATED SUPPORT INFORMATION MIGHT BE OUTDATED OR DEPRECATED

On 28/01/2009 at 16:39, xxxxxxxx wrote:

User Information:
Cinema 4D Version:   11 
Platform:   Windows  ;   
Language(s) :     C++  ;

---------
Hey everybody,

I don't want to be one of those people who post code on forums and say "fix this!" as if people were obliged to help out. So instead, I'll ask nicely =P

Seriously though, I'm feeling very uncertain about the code I'm writing for my plugins. The main reason is that there is no "how to start writing plugins for C4D" tutorial that I could find. I'm reading every bit of introductory information I can find on the SDK, but there are still some areas that are vague in my mind.

I'm working on a plugin called MECO (stands for Make-Editable, Connect, Optimize) which boils down all those commands that you need to apply to a NURBS object to get it to a point where it can behave like any other polygonal object. I'm going to release this plugin for free and also open-source. Thus, I'd like it to be as robust as possible.

Would anyone be willing to look at my code and point out the errors of my way? It's commented well so you shouldn't have any problem understanding what I'm doing (especially the experts here).

This is a Command Plugin. Code from the Execute function follows:

> <code>
>      //Gotta stop all threads first since we're modifying the active document
>      StopAllThreads();
>      //Array to hold selected objects
>      AtomArray* selectedObjs = AtomArray::Alloc();
>      //Fill array with selected objects
>      doc->GetActiveObjects(*selectedObjs, TRUE);
>      
>      //Check if any objects are selected
>      if(selectedObjs->GetCount() == 0)
>      {
>           //If not, free the array and then return
>           AtomArray::Free(selectedObjs);
>           return FALSE;
>      }
>
>      //Create Modeling Command Data
>      ModelingCommandData cd;
>      cd.doc = doc;
>      //Set the objects to be changed
>      cd.arr = selectedObjs;
>
>      if(SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))
>      {
>           //Command was successful
>           //Insert the resulting objects into the document
>           for(int i=0; i<cd.result->GetCount(); i++)
>           {
>                doc->InsertObject((BaseObject* ) (cd.result->GetIndex(i)), NULL, NULL, FALSE);
>           }          
>      } else {
>           //Command failed
>           //Free the array and then return
>           AtomArray::Free(selectedObjs);
>           return FALSE;
>      }
>
>      //Free the previous array as it's not valid anymore
>      AtomArray::Free(selectedObjs);
>      
>      //Select these objects
>      //Select the first one seperately to create a new selection with SELECTION_NEW
>      doc->SetSelection((BaseList2D* ) (cd.result->GetIndex(0)), SELECTION_NEW);
>      //Now add the others
>      for(int i=1; i<cd.result->GetCount(); i++)
>      {
>           doc->SetSelection((BaseList2D* ) (cd.result->GetIndex(i)), SELECTION_ADD);
>      }
>      
>      //Next step is to Connect these objects
>      //Array to hold new selected objects
>      AtomArray* selectedObjs_2 = AtomArray::Alloc();
>      //Fill array with new selected objects
>      doc->GetActiveObjects(*selectedObjs_2, TRUE);
>
>      //Create new Modeling Command Data
>      ModelingCommandData cd_2;
>      cd_2.doc = doc;
>      //Set the objects to be changed
>      cd_2.arr = selectedObjs_2;
>
>      if(SendModelingCommand(MCOMMAND_JOIN, cd_2))
>      {
>           //Command was successful
>           //Insert the resulting objects into the document
>           for(int i=0; i<cd_2.result->GetCount(); i++)
>           {
>                doc->InsertObject((BaseObject* ) (cd_2.result->GetIndex(i)), NULL, NULL, FALSE);
>           }
>      } else {
>           //Command failed
>           //Free the new array and then return
>           AtomArray::Free(selectedObjs_2);
>           return FALSE;
>      }
>
>      //Free the previous array as it's not valid anymore
>      AtomArray::Free(selectedObjs_2);
>
>      //Delete the original objects (which are selected) as they are not removed automatically
>      //Array to hold new selected objects
>      AtomArray* selectedObjs_3 = AtomArray::Alloc();
>      //Fill array with new selected objects
>      doc->GetActiveObjects(*selectedObjs_3, TRUE);
>      for(int i=0; i<selectedObjs_3->GetCount(); i++)
>      {
>           BaseObject* bobj = (BaseObject* ) selectedObjs_3->GetIndex(i);
>           bobj->Remove();
>           BaseObject::Free(bobj);
>      }
>      
>      //Free the previous array as it's not valid anymore
>      AtomArray::Free(selectedObjs_3);
>
>      //Next step is to Optimize points on the objects
>      //Create new Modeling Command Data
>      ModelingCommandData cd_3;
>      cd_3.doc = doc;
>      //Also need a Base Container to pass Optimize command data
>      BaseContainer bc;
>      bc.SetReal(MDATA_OPTIMIZE_TOLERANCE, 0.01);
>      bc.SetReal(MDATA_OPTIMIZE_POINTS, TRUE);
>      bc.SetReal(MDATA_OPTIMIZE_POLYGONS, TRUE);
>      bc.SetReal(MDATA_OPTIMIZE_UNUSEDPOINTS, TRUE);
>      //Link the Base Container to the Modeling Command Data
>      cd_3.bc = &bc;
>      //Set the objects to be changed
>      cd_3.arr = cd_2.result;
>
>      if(SendModelingCommand(MCOMMAND_OPTIMIZE, cd_3))
>      {
>           doc->Message(MSG_UPDATE, NULL);
>           //Optimize command doesn't return anything.
>      } else {
>           //Command failed
>           return FALSE;
>      }
>
>      //Force redraw
>      EventAdd(EVENT_FORCEREDRAW);
>
>      return TRUE;
> </code>

Things I'm aware of:
-There's no UNDO support as of now.

-It actually does what it's supposed to do.

Things I'm worried about:
-Do I need to send UPDATE messages to the active document or the changed/inserted objects after every modeling command?

-I actually don't quite know what messages are for in the first place? Any pointers to useful information regarding messages would be very useful.

-There's a bug with my command that doesn't make any sense to me. If you create a Rectangle spline, put it under an Extrude NURBS, then rotate the Extrude NURBS an arbitrary amount of degrees, then run my command on the Extrude NURBS object, it creates the new object in the original rotation of the Extrude NURBS and not the latest and correct rotation.

Thank you very much for reading this post,
Yilmaz Kiymaz

THE POST BELOW IS MORE THAN 5 YEARS OLD. RELATED SUPPORT INFORMATION MIGHT BE OUTDATED OR DEPRECATED

On 28/01/2009 at 17:03, xxxxxxxx wrote:

1. If you are inserting the results into the document, yes, you need to send op->Message(MSG_UPDATE) per op as well as one EventAdd() for the changes to occur and be recognized by the document.

2. Messages are sent for specific events. For non-dialog things (objects, tags, materials, etc.) they are delineated in the Message() function of C4DAtom which is the ultimate base class of all of these other classes.

3. Ah, the SendModelingCommand() rollercoaster fun ride. :) Some commands require that you impart the original object's global matrix (op->GetMg()) to the global matrix or points (if a point object derivative). Some apply it automatically and you want to counteract it by setting the global matrix as a unit matrix (op->SetMg(unitMatrix)). It really requires that you go through each SendModelingCommand() step and ensure that your results are as expected. I ended up inserting each step into the document to see this.

Addendum. Instead of hard-allocating AtomArrays, you could simply use AutoAlloc<AtomArray> so that it is automatically freed when it goes out of scope. To reuse in the same scope, simply call atomarray->Flush() between uses. I would also reuse the ModelingCommandData by clearing variables and resetting those of relevance. All of this saves stack/memory space and may provide speed increases.

THE POST BELOW IS MORE THAN 5 YEARS OLD. RELATED SUPPORT INFORMATION MIGHT BE OUTDATED OR DEPRECATED

On 12/02/2009 at 08:49, xxxxxxxx wrote:

Hey there,

I didn't mean to reply so late. I've taken all of Kuroyume's suggestions to heart and worked them in. However, I'm still struggling with getting proper Undo support in there.

As I mentioned before, this is an open-source plugin. So I'm going to make the source files available. Right about... now!: -link-
One of the main reasons I'm making this open-source is that not only I but other can also learn from my mistakes and that it makes it easier for other people to help me out with completing this plugin.

Now, onto current issues. I am completely baffled as to how I should add Undo support in there. I've searched the forums for the proper way to do it but it's really hard to find consistent information that way. I tried calling StartUndo() before any of my Modeling Commands, then using AddUndo() after every change I make to the scene in the Modeling Commands, and finally calling EndUndo() after all three Modeling Commands have completed. This did not work. When I undo, the resulting object of my plugin doesn't go away, and sometimes it even screws up the Undos of the application itself (such as creating a primitive, that undo dissapears somehow).

I read in a post that StartUndo() gets called automatically before any Execute method of plugins and I tried to code with that in mind, but still no luck.

If anyone would be kind enough to take a look at my source code and let me know what I'm doing wrong, that would be wonderful. Also, I was very generous with commenting the code so you should have very little trouble understanding what's going on :)

As a side note, did anyone ever mention the idea of creating a Wiki for C4D plugin creation? It seems to me that beginners could really benefit from this. If this was not an idea that was shot down earlier, I'll post a topic about this to open it up to discussion.

Thank you for your time,
Yilmaz Kiymaz

THE POST BELOW IS MORE THAN 5 YEARS OLD. RELATED SUPPORT INFORMATION MIGHT BE OUTDATED OR DEPRECATED

On 12/02/2009 at 16:49, xxxxxxxx wrote:

Make sure that you are not letting SendModelingCommand() create automatic undos for you. This happens, for instance, when the result is set to be automatically placed into the document. NO! Don't do that. Get the result, add it to the document yourself, and add an undo.

If you are doing a series of SendModelingCommands() and only the last result is being added to the document, you only need to do StartUndo(), AddUndo(UNDO_NEW, op), EndUndo() after inserting it into the document.

Remember that some of the commands operate directly on the input object (like triangulation). In this case, unless you really want the original triangulated, make a clone and do the work on it. This will avoid odd undo situations.