Skip to content

Conversation

@g-abilio
Copy link

This PR adds the node processing status dynamic that was present in nodeeditor v1. It has now been updated to seamlessly adapt to nodeeditor v3. It also contains an example to illustrate this updated dynamic.

@g-abilio g-abilio requested a review from tatatupi July 18, 2025 15:05
Copy link

@tatatupi tatatupi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The icon is too small:

Image

And I didn't understand why you included the struct NodeProcessingStatus inside the NodeDelegateModel. I removed it from it, leaving the struct at the same "level" as NodeValidationState.

However, I don't know if it needs to be a struct, as the Validation State, and not directly an enum class. Why did you choose the struct?

@tatatupi
Copy link

I have fixed the conflicts, however. Soon it will be available for merge request in paceholder

@g-abilio g-abilio requested a review from tatatupi September 15, 2025 17:50
@tatatupi
Copy link

The UI is better, but the example is not yet very good.
Computation nodes (e.g., division, multiplication) should default to an empty icon.

I also do not understand the logic behind random generation. Sometimes it throws an error. Sometimes it doesn't.

The image shows a processing icon for a node that has empty input.
Nodes with empty input showing a valid icon
And a node with an error showing a valid icon.

We should have a very functional example before suggesting a PR.

image

@tatatupi
Copy link

Other examples are also displaying the processing status when they shouldn't.
As we talked, the default option should omit the processing status functionality. It should be optional for the developer to implement it or not.

image image

@g-abilio
Copy link
Author

Now, to address these example issues, the following changes were made:

  1. No default value for the processing status in the Node Delegate Template, as the processing status icon issue that appeared in other examples was triggered by this default value.
  2. Empty processing status appears until all inputs for the node are provided (in the example, the two numeric inputs for the Random Number Generator node).

And, to explain the appearance of the Error processing status, there is a conditional statement that change the control flow when the first input for the node (the top one) is greater than the second input (the bottom one), so that the Error processing status can also be displayed and seen as a processing status option by the PR reviewer. The conditional statement is as follows:

double a = n1->number();
double b = n2->number();
                
if (a > b) {
    SetNodeProcessingStatus(QtNodes::NodeProcessingStatus::Failed);

    emit requestNodeUpdate();
    return;
}

So, the goal is to keep a random number being generated in the range [a,b] ([n1, n2]), as described in the Random Number node class docstring. These control flow details will be explained in the final nodeeditor PR!

@g-abilio g-abilio merged commit 21b55ac into master Oct 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants