AmplifyCreations / AmplifyShaderEditor-Feedback

4 stars 0 forks source link

Break OnNodeRepaint into smaller chunks #330

Open Nestorboy opened 3 months ago

Nestorboy commented 3 months ago

As it stands, the OnNodeRepaint function in ParentNode is quite hefty, as it handles drawing the entirety of a node, from the header to ports the preview. All the code for this is currently inside of that one function and it's got comments to denote sections relating to the specific parts.

Recently I decided to use Harmony to introduce new behaviour to how ports are drawn, but I soon found myself copying over 200 lines of code from the OnNodeRepaint from ParentNode and WireNode functions in ASE in order to replace a handful of lines whilst retain the old behaviour. Whilst I've got everything working as intended, my main concern now is redistribution of the code, even though I've refactored a majority of it.

Whilst my particular use-case is not common or something that I'd expect support for, the underlying issue is that the OnNodeRepaint function has way too much going on in it, and it could use some spring-cleaning. Even if the code to draw the different parts of a node isn't re-used in different locations, it would be much more readable and concise if the OnNodeRepaint function acted more like an overview of its responsibilities and just called other functions that handled drawing the specific parts. This in turn would make it much faster to see what it draws but also introduce the ability to more easily replace specific parts of the drawing loop using something like Harmony.

Here's some quick pseudo-code to get the point across:

OnNodeRepaint()
{
    DrawBackground();
    DrawHeader();
    DrawPorts();
    DrawPreview();
    DrawSelection();
}

DrawPorts()
{
    foreach (inPort in inputPorts)
        DrawPort(inPort);
    foreach (outPort in outputPorts)
        DrawPort(outPort);
}

DrawPort(Port port) { ... }