Solved Generators, Materials, Undo's, Oh My!

Hey Folks!
Two questions followed by the context.

• Where's the best place to add/update materials when those changes are initiated from the parameters of a generator object plugin? MSG_DESCRIPTION_POSTSETPARAMETER? MSG_DESCRIPTION_CHECKUPDATE? SetDParameter()? Somewhere else?

• Is it correct that in some instances the Start/EndUndo() calls aren't necessary, such as in the case of the previously mentioned messages/functions, since they are received/executed between MSG_DESCRIPTION_INITUNDO and MSG_DESCRIPTION_USERINTERACTION_END?

I have a generator plugin written in python that has a few parameters that are meant to deal with generating/adjusting materials (a filename parameter and a couple of combo boxes). Grouping the creation/adjusting of materials with their parameter change into a single undo was proving difficult until I stumbled upon the above info on the exception to the "rule of undo's" (must call Start/EndUndo()).

Then there were the crashes and the errors related to baseundo.cpp (I've included those below).

Critical_Log
	{
		[12:41:19] CRITICAL: Stop [baseundo.cpp(1857)]
		[12:41:19] CRITICAL: Stop [baseundo.cpp(1956)]
		[12:41:19] CRITICAL: Stop [baseundo.cpp(1956)]
		[12:41:19] CRITICAL: Stop [baseundo.cpp(1897)]
		[12:49:45] CRITICAL: Stop [baseundo.cpp(1857)]
		[12:49:45] CRITICAL: Stop [baseundo.cpp(1956)]
		[12:49:45] CRITICAL: Stop [baseundo.cpp(1956)]
		[12:49:45] CRITICAL: Stop [baseundo.cpp(1897)]
	}

I had myself convinced it was all related to those material affecting parameters and that I had misunderstood the way undo's work with parameter changes, which led me to the two questions above. In the end it didn't even have anything to do with any of that, just something stupid I was doing in Execute() until I rediscovered the threading information page in the sdk. So the problem, technically, is all fixed, but after all the experimenting while on the "wrong track" I'm left with 3 different places where I could put the code for those material changes and I'm just not sure which would be best.

Thanks!
Kevin

@zipit said in Generators, Materials, Undo's, Oh My!:

Hi,

I did not read all your code, but first of all you should replace statements like doc = c4d.documents.GetActiveDocument() with something like doc = node.GetDocument(). The methode is BaseList2D.GetDocument, you need to retrieve the document your node is attached to, not the active document. The reason is that nodes are not only being execute in the active document, but also other documents, for rendering the document is getting cloned for example. In these cases you would operate on the wrong document.

Apart from this: I am a bit surprised that you actually managed to sneak in some operations into an Undo action of the node with MSG_DESCRIPTION_POSTSETPARAMETER, sorry for my misleading info on that, but there is not much to say about this.

This is very likely not intended by MAXON, so you will probably neither get advice nor support for this approach. The context seems somewhat safe, i.e. the chance that you accidentally add your operation to some other Undo context seems low, but since this is not documented and probably also not intended, there are no guarantees, especially considering the rather fragile nature that Undo stacks often have. I would encapsulate your AddUndo logic blocks by a condition that ensures that they are only executed on the main thread, to avoid any possible major f*** ups. Other than that there is not much that I would do, aside from not doing this at all.

Cheers,
zipit

Yes, of course... I'm an idiot, haha. I think I even noticed that while tooling around the forums and totally spaced on making the change to my document calls.

Ok, I found two more bits of information that make me feel more comfortable in my approach. I keep forgetting to also reference the C++ sdk when writing a plugin in python lol. First link is from there... the "Undo System Manual", actually... would certainly be nice if that made its way into the python docs;) Second link is Maxime confirming that wrapping other node changes with the built-in parameter undo is appropriate, including a link to an example that uses MSG_DESCRIPTION_POSTSETPARAMETER. While that example ends up sending a MSG_DESCRIPTION_COMMAND to another node, it was shared in the context of, and as a solution for, grouping other undo's into the built-in parameter undo's.

https://developers.maxon.net/docs/Cinema4DCPPSDK/html/page_manual_undo.html

https://plugincafe.maxon.net/topic/12493/undo-for-a-tagdata/2

Hopefully, if I'm misunderstanding any of this or there's some other detail that's still missing from the equation, a dev will come along and correct the record. Otherwise, I'm feeling pretty confident in marking this one as solved.

Thanks for all your help Zipit! You certainly helped me find some holes and I'll be sure to take your advice and shore this up so it's as formidable as Helm's Deep... ok, bad example... Helm's Deep without that blasted culvert!

Thanks!
Kevin

Hi,

I think you are having more of a conceptual problem than a technical problem.

@kvb said

• Where's the best place to add/update materials when those changes are initiated from the parameters of a generator object plugin? MSG_DESCRIPTION_POSTSETPARAMETER? MSG_DESCRIPTION_CHECKUPDATE? SetDParameter()? Somewhere else?

While it is technically possible to do what you are asking for, there is no conceptually meaningful answer to this question. If you would add an Undo for the modification of material M after the user has modified a parameter in your generator G, you would have set up yourself for an ambiguous state of your scene. Because after that you have the modified nodes in G' and M' in your scene, but when the user would invoke an Undo, you would have G' and M in your scene, i.e. there would be two material states for one generator state. This would contradict the concept of Undos.

• Is it correct that in some instances the Start/EndUndo() calls aren't necessary, such as in the case of the previously mentioned messages/functions, since they are received/executed between MSG_DESCRIPTION_INITUNDO and MSG_DESCRIPTION_USERINTERACTION_END?

No, Cinema won't (intentionally) call a node in the context of a non-finalized undo-step, but nodes also have builtin Undos, so it is almost never a good idea to use Undos in a node (even when respecting the threading limitations of Undos).

You should design your plugin in such way that:

• Each material node is only being targeted by exactly one generator node (but a generator node could be in a 1:N relation)
• The generator node either locks onto the material node(s) upon instantiation or creates the required material nodes upon instantiation.
• The state of the material node(s) M is determined by a function that is only dependent on their generator G (i.e. M=f(G)).
  • This would keep your scene state unambiguous.
  • You could execute this logic from GetVirtualObjects (node modifications are allowed from a threaded context)
  • You would not have to worry about Undos, since invoking an Undo on parameter modification of your generator node would automatically update the material(s), since they are a function of that node.

Cheers,
zipit

Hey Zipit, thanks for the detailed reply! It really helps my understanding of all the little intricacies of the sdk;)

Unfortunately, I don't think I can eliminate all of the undo's I need using GVO. I can't anticipate all the materials that will be needed given that their creation is based on user input through a filename parameter on my generator, so I can't create them during instantiation. That leaves me very few options since I can't create them during GVO.

So how bad are those undo's? Because regardless of the material creation aspect I still have file i/o and os function calls to be concerned about (allowed during execution but I think os function calls are discouraged?). If those are fine I suppose I have a workaround if it's possible to create materials in the same manner as the virtual objects. The user wouldn't have access to them in the current scene, but I could open up the material freedom through specific override links. A bit more limiting than my current implementation, but might as well consider it if it's possible.

I also have a couple of buttons that create a camera and render settings, those also include undo's (reacted to in MSG_DESCRIPTION_COMMAND, full start->add->end blocks since it lies outside of the built in node undo's).

Thanks!
Kevin

Hello,

I would like to understand what's the workflow behind your tool. Maybe you could jump on c++ and create a scenehook. (not hard at all)
what kind of OS function are you calling ?

For your next threads, please help us keeping things organised and clean.

• Q&A New Functionality.
• How to Post Questions especially the tagging part.

I've added the tags and marked this thread as a question so when you considered it as solved, please change the state :)

Hi,

maybe I was unclear on that, but you cannot incorporate your scene modifications into an Undo created by Cinema 4D for the modification of a parameter of a node. You will always end up with two Undos, one created by Cinema 4D for the parameter modification and one created by you for the scene modifications. Because of the ambiguous scene state stuff I described above there is also the chance you might send the user into a loop ("... your generator node adds a material and an Undo for it, user invokes Undo, your generator is still in the same state, so it adds a material and an Undo for it, ...").

On the material creation: If you need an unknown amount of up to n materials, but know n, you could just always create n materials and unhide/use them on a as needed basis. But you do not have to create all materials on instantiation. Technically you could also delay that by escaping the threaded context of GVO by for example using messages and a MessageData plugin. But see the last paragraph on that.

On invoking OS functionalities from a threaded context: There isn't anything special about it in Cinema, at least I am not aware of anything special. It also applies to all data access for NodeData methods that are executed asynchronously, not just calls to the OS. If you want to access the same block of data from multiple asynchronously executed functions, you will need a semaphore for doing that safley.

I would also keep in mind that generators have intentionally been designed for asynchronous execution and to encapsulate a part of a scene graph. So if you find yourself constantly bending and circumventing these concepts, you might be better of with another plugin type, like CommandData, ToolData or MessageData.

Cheers,
zipit

@m_magalhaes said in Generators, Materials, Undo's, Oh My!:

Hello,

I would like to understand what's the workflow behind your tool. Maybe you could jump on c++ and create a scenehook. (not hard at all)
what kind of OS function are you calling ?

For your next threads, please help us keeping things organised and clean.

• Q&A New Functionality.
• How to Post Questions especially the tagging part.

I've added the tags and marked this thread as a question so when you considered it as solved, please change the state :)

Thanks! And apologies for not reviewing the forum rules, been a hot minute since I've posted and it was one of those "You've revised this a dozen times, if you don't post it now you'll be revising it forever" kind of things.

Would love to be able to utilize a scenehook! But it's way too late to re-write this in C++:(

@zipit said in Generators, Materials, Undo's, Oh My!:

Hi,

maybe I was unclear on that, but you cannot incorporate your scene modifications into an Undo created by Cinema 4D for the modification of a parameter of a node. You will always end up with two Undos, one created by Cinema 4D for the parameter modification and one created by you for the scene modifications. Because of the ambiguous scene state stuff I described above there is also the chance you might send the user into a loop ("... your generator node adds a material and an Undo for it, user invokes Undo, your generator is still in the same state, so it adds a material and an Undo for it, ...").

On the material creation: If you need an unknown amount of up to n materials, but know n, you could just always create n materials and unhide/use them on a as needed basis. But you do not have to create all materials on instantiation. Technically you could also delay that by escaping the threaded context of GVO by for example using messages and a MessageData plugin. But see the last paragraph on that.

On invoking OS functionalities from a threaded context: There isn't anything special about it in Cinema, at least I am not aware of anything special. It also applies to all data access for NodeData methods that are executed asynchronously, not just calls to the OS. If you want to access the same block of data from multiple asynchronously executed functions, you will need a semaphore for doing that safley.

I would also keep in mind that generators have intentionally been designed for asynchronous execution and to encapsulate a part of a scene graph. So if you find yourself constantly bending and circumventing these concepts, you might be better of with another plugin type, like CommandData, ToolData or MessageData.

Cheers,
zipit

But that's the thing... it is working! I'm getting perfect undo behavior, without any errors. I'm not creating a new undo block, I'm slipping extra AddUndo() calls between (and only ever between) the built in Start/EndUndo() calls of the node. I only get the behavior you describe (needing two or more undo's) if I add my own Start/EndUndo() calls.

It's like, if this were a c++ problem it would be like I'm asking about some warnings I'm getting at compile. Usually fine, but definitely better if there are as few of them as possible. Except the warnings are coming from my head in the form of second guessing myself lol. I'm just here making sure I'm doing as little "wrong" as possible;) I can definitely see why Undo's are highly discouraged in node based plugins, as you can get yourself in trouble very easily if you're not careful. But I think I'm doing it safely, and even if it's not expressly enumerated in the sdk, according to its design... I think (why I'm here lol).

The only os calls I'm making are os.path.basename() and os.path.exists(). As far as the potential number of materials needed, n > the number of dummy materials I feel comfortable making per instance of my plugin object:)

Thanks!
Kevin

Here's some simplified code that hopefully illustrates how I'm approaching this. It's the function where I load and store the relevant image data into the generator's basecontainer (for later use in GVO), create a material, and make further alterations to it. There's also the Message function where I'm slipping my undo's into the node's built-in undo block. Is anything about this code wrong? Like I said, everything is working perfectly, but I know that just because something works doesn't mean it's necessarily correct. But the logic seems correct to me. C4D is giving me an open undo block in a function where I'm allowed to make scene alterations, where the preferred option of "avoid the undo's" results the most broken undo behavior, and then C4D conveniently closes the undo for me. Am I understanding this wrong?

Thanks!
Kevin

def LoadImageData(self, node):
    """ This function is only ever called from within an
    open undo block in non-threaded functions.
    Stores image data into the generator's basecontainer
    and creates a material and inserts the image as a shader """
    doc = c4d.documents.GetActiveDocument()
    
    # Load the texture
    image_path = node[c4d.ID_USER_PROVIDED_IMAGE_FILE]
    if not image_path:
        return False
    bc = c4d.BaseContainer()
    bc.SetFilename(c4d.LOADTEXTURE_FILENAME, image_path.encode('utf-8'))
    tex = c4d.modules.bodypaint.SendPainterCommand(c4d.PAINTER_LOADTEXTURE, doc=doc, tex=None, bc=bc)
    if tex is False:
        return False

    # Save relevant image data to generator's basecontainer and close texture.
    doc.AddUndo(c4d.UNDOTYPE_CHANGE, node)
    node[c4d.ID_IMAGE_WIDTH] = float(tex.GetBw())
    node[c4d.ID_IMAGE_HEIGHT] = float(tex.GetBh())
    node[c4d.ID_IMAGE_NAME] = os.path.basename(image_path)    
    c4d.modules.bodypaint.SendPainterCommand(c4d.PAINTER_FORCECLOSETEXTURE, doc=doc, tex=tex, bc=c4d.BaseContainer())

    # Check for active material or create and set if None.        
    mat = node[c4d.ID_MATERIAL_LINK]
    if not mat:
        mat = c4d.Material()
        mat.SetName(node[c4d.ID_IMAGE_NAME])
        mat[c4d.MATERIAL_USE_REFLECTION] = False
        doc.InsertMaterial(mat)
        doc.AddUndo(c4d.UNDOTYPE_NEW, mat)
        node[c4d.ID_MATERIAL_LINK] = mat     
    
    # Set main image channel
    if node[c4d.ID_MAIN_IMAGE_CHANNEL] == c4d.ID_MAIN_IMAGE_CHANNEL_COLOR:
        chanID = c4d.MATERIAL_COLOR_SHADER
        mat[c4d.MATERIAL_USE_COLOR] = True
        mat[c4d.MATERIAL_USE_LUMINANCE] = False
    else:
        chanID = c4d.MATERIAL_LUMINANCE_SHADER
        mat[c4d.MATERIAL_USE_COLOR] = False
        mat[c4d.MATERIAL_USE_LUMINANCE] = True

    # Load the image into the channel's shader
    bitmap_shader = c4d.BaseShader(c4d.Xbitmap)
    bitmap_shader[c4d.BITMAPSHADER_FILENAME] = str(node[c4d.ID_USER_PROVIDED_IMAGE_FILE])
    mat[chanID] = bitmap_shader
    mat.InsertShader(bitmap_shader)
    doc.AddUndo(c4d.UNDOTYPE_NEW, bitmap_shader)
    node[c4d.ID_IMAGE_LOADED] = True
    
    return True

def Message(self, node, type, data):
    doc = c4d.documents.GetActiveDocument()
    
    """ MSG_DESCRIPTION_INITUNDO and MSG_DESCRIPTION_USERINTERACTION_END included
        to illustrate how MSG_DESCRIPTION_POSTSETPARAMETER is received within
        an open undo block """

    if type == c4d.MSG_DESCRIPTION_INITUNDO:
        """ This message initiates a StartUndo() call. Marking the start of the built-in parameter undo block """
    
    if type == c4d.MSG_DESCRIPTION_POSTSETPARAMETER:
        """ This message is received between MSG_DESCRIPTION_INITUNDO and MSG_DESCRIPTION_USERINTERACTION_END.
            Meaning it takes place during the open built-in parameter undo block. All undo's initiated
            from within this message will be grouped with their respective parameter changes """
        
        """ Filename input on the generator object. Calls a function to load the image file, creating materials in the process.
            Undo's occur within LoadImageData() """
        if data and data['descid'][0].id == c4d.ID_USER_PROVIDED_IMAGE_FILE:
            if os.path.exists(node[c4d.ID_USER_PROVIDED_IMAGE_FILE]):
                if self.LoadImageData(node, True) == False
                    return False
                node.SetDirty(c4d.DIRTYFLAGS_DATA)
            else:
                return False
        
        """ Combo box to adjust material parameters """
        if data and data['descid'][0].id == c4d.ID_MAIN_IMAGE_CHANNEL:
            if node[c4d.ID_IMAGE_LOADED] == True:
                mat = node[c4d.ID_MATERIAL_LINK]
                if mat:
                    doc.AddUndo(c4d.UNDOTYPE_CHANGE, mat)   
                    if node[c4d.ID_MAIN_IMAGE_CHANNEL] == c4d.ID_MAIN_IMAGE_CHANNEL_COLOR:
                        mat[c4d.MATERIAL_USE_COLOR] = True
                        mat[c4d.MATERIAL_USE_LUMINANCE] = False
                    else:
                        mat[c4d.MATERIAL_USE_COLOR] = False
                        mat[c4d.MATERIAL_USE_LUMINANCE] = True       
                else:
                    return False       
        
        """ Combo box parameter that may create and add a camera to the scene """                   
        if data and data['descid'][0].id == c4d.ID_TEXTURE_PROJECTION:
            if node[c4d.ID_TEXTURE_PROJECTION] == c4d.ID_TEXTURE_PROJECTION_CAMERA_MAPPING:
                if node[c4d.ID_LINKED_CAMERA] is None:
                    cam = c4d.BaseObject(c4d.Ocamera)
                    image_name = node[c4d.ID_IMAGE_NAME]
                    if image_name and node[c4d.ID_IMAGE_LOADED] == True:
                        cam.SetName(image_name + " cam")
                        doc.InsertObject(cam, parent = node)
                        doc.AddUndo(c4d.UNDOTYPE_NEW, cam)
                        doc.AddUndo(c4d.UNDOTYPE_CHANGE, node)
                        node[c4d.ID_PLANESMART_CAMERA] = cam
                        fov = cam[c4d.CAMERAOBJECT_FOV]
                        cam.SetAbsPos(c4d.Vector(0, 0, -(node[c4d.ID_IMAGE_WIDTH] / 2 / math.tan(fov / 2)))) 
                        node.ChangeNBit(c4d.NBIT_OM1_FOLD, c4d.NBITCONTROL_SET)
                        node.SetDirty(c4d.DIRTYFLAGS_DATA)
                    else:
                        return False

    elif type == c4d.MSG_DESCRIPTION_USERINTERACTION_END:
        """This message initiates an EndUndo() call. Marking the end of the built-in parameter undo block"""       

    elif type == c4d.MSG_DESCRIPTION_COMMAND:
        """ Example of how I'm handling button input with undo's.  In this case creating render settings.
            Uses a complete Undo block since it's received outside of the built-in parameter undo block. """
        if data and data['id'][0].id == c4d.ID_CREATE_RENDER_SETTINGS:
            doc.StartUndo()
            image_name = node[c4d.ID_IMAGE_NAME]
            if image_name node[c4d.ID_IMAGE_LOADED] == True:
                # Returns a tuple with a bool for pre-existance of render settings and the RenderData
                rd_exists, rd = SearchRenderSettings(rs, image_name)
                if rd_exists == False:
                    rd = doc.GetActiveRenderData().GetClone()
                rd_bc = rd.GetDataInstance()
                rd_bc[c4d.RDATA_XRES] = node[c4d.ID_IMAGE_WIDTH]
                rd_bc[c4d.RDATA_YRES] = node[c4d.ID_IMAGE_HEIGHT]
                # Have to set film aspect manually, not sure why
                rd_bc[c4d.RDATA_FILMASPECT] = node[c4d.ID_IMAGE_WIDTH] / node[c4d.ID_IMAGE_HEIGHT]
                if rd_exists == False:
                    # We don't want to inadvertently overwrite previous renders!
                    rd_bc[c4d.RDATA_PATH] = ""
                    rd.SetName(image_name)
                    doc.InsertRenderDataLast(rd)
                    doc.AddUndo(c4d.UNDOTYPE_NEW, rd)
                doc.SetActiveRenderData(rd)
            else:
                return False
            doc.EndUndo()

    return True

Hi,

I did not read all your code, but first of all you should replace statements like doc = c4d.documents.GetActiveDocument() with something like doc = node.GetDocument(). The methode is BaseList2D.GetDocument, you need to retrieve the document your node is attached to, not the active document. The reason is that nodes are not only being execute in the active document, but also other documents, for rendering the document is getting cloned for example. In these cases you would operate on the wrong document.

Apart from this: I am a bit surprised that you actually managed to sneak in some operations into an Undo action of the node with MSG_DESCRIPTION_POSTSETPARAMETER, sorry for my misleading info on that, but there is not much to say about this.

This is very likely not intended by MAXON, so you will probably neither get advice nor support for this approach. The context seems somewhat safe, i.e. the chance that you accidentally add your operation to some other Undo context seems low, but since this is not documented and probably also not intended, there are no guarantees, especially considering the rather fragile nature that Undo stacks often have. I would encapsulate your AddUndo logic blocks by a condition that ensures that they are only executed on the main thread, to avoid any possible major f*** ups. Other than that there is not much that I would do, aside from not doing this at all.

Cheers,
zipit

@zipit said in Generators, Materials, Undo's, Oh My!:

Hi,

I did not read all your code, but first of all you should replace statements like doc = c4d.documents.GetActiveDocument() with something like doc = node.GetDocument(). The methode is BaseList2D.GetDocument, you need to retrieve the document your node is attached to, not the active document. The reason is that nodes are not only being execute in the active document, but also other documents, for rendering the document is getting cloned for example. In these cases you would operate on the wrong document.

Apart from this: I am a bit surprised that you actually managed to sneak in some operations into an Undo action of the node with MSG_DESCRIPTION_POSTSETPARAMETER, sorry for my misleading info on that, but there is not much to say about this.

This is very likely not intended by MAXON, so you will probably neither get advice nor support for this approach. The context seems somewhat safe, i.e. the chance that you accidentally add your operation to some other Undo context seems low, but since this is not documented and probably also not intended, there are no guarantees, especially considering the rather fragile nature that Undo stacks often have. I would encapsulate your AddUndo logic blocks by a condition that ensures that they are only executed on the main thread, to avoid any possible major f*** ups. Other than that there is not much that I would do, aside from not doing this at all.

Cheers,
zipit

Yes, of course... I'm an idiot, haha. I think I even noticed that while tooling around the forums and totally spaced on making the change to my document calls.

Ok, I found two more bits of information that make me feel more comfortable in my approach. I keep forgetting to also reference the C++ sdk when writing a plugin in python lol. First link is from there... the "Undo System Manual", actually... would certainly be nice if that made its way into the python docs;) Second link is Maxime confirming that wrapping other node changes with the built-in parameter undo is appropriate, including a link to an example that uses MSG_DESCRIPTION_POSTSETPARAMETER. While that example ends up sending a MSG_DESCRIPTION_COMMAND to another node, it was shared in the context of, and as a solution for, grouping other undo's into the built-in parameter undo's.

https://developers.maxon.net/docs/Cinema4DCPPSDK/html/page_manual_undo.html

https://plugincafe.maxon.net/topic/12493/undo-for-a-tagdata/2

Hopefully, if I'm misunderstanding any of this or there's some other detail that's still missing from the equation, a dev will come along and correct the record. Otherwise, I'm feeling pretty confident in marking this one as solved.

Thanks for all your help Zipit! You certainly helped me find some holes and I'll be sure to take your advice and shore this up so it's as formidable as Helm's Deep... ok, bad example... Helm's Deep without that blasted culvert!

Thanks!
Kevin

hello,
thanks a lot @zipit for all of your time :)

@kvb
Maybe there are some cases where this will go wrong but we don't see them at the moment :)
We talked about it this morning and while we can't guaranty it's going to work we can't said it's bad.

I'm not a big fan of the workflow but I'm happy if you are.

Cheers,
Manuel

@m_magalhaes said in Generators, Materials, Undo's, Oh My!:

hello,
thanks a lot @zipit for all of your time :)

@kvb
Maybe there are some cases where this will go wrong but we don't see them at the moment :)
We talked about it this morning and while we can't guaranty it's going to work we can't said it's bad.

I'm not a big fan of the workflow but I'm happy if you are.

Cheers,
Manuel

Thanks Manuel, I'll be sure to report any issues I encounter. I tried to mark your post as the correct response but mistakenly used the options sub-menu above your post, which marked my post as correct and I don't seem to be able to change it.

Thanks!
Kevin

the whole thread is the answer anyway :)