juce-framework / JUCE

JUCE is an open-source cross-platform C++ application framework for desktop and mobile applications, including VST, VST3, AU, AUv3, LV2 and AAX audio plug-ins.
https://juce.com
Other
6.31k stars 1.67k forks source link

[Bug]: AudioProcessorGraph unexpectedly mutates a node UID of 0 instead of disallowing it #1310

Open tristan00b opened 7 months ago

tristan00b commented 7 months ago

Detailed steps on how to reproduce the bug

Call AudioProcessorGraph::addNode with NodeID{ 0 } for the second argument. A new node will be inserted into the graph with the UID set to ++(lastNodeID.uid). (It's the same for both the master and develop branches.)

What is the expected behaviour?

If an ID of 0 is not permitted, then I would expect the call to fail in some fashion.

In my case, I was trying to zero-index the node ID's so as to have them correspond to an enum I was using to index into an array. The next call to addNode with an argument of NodeID{ 1 } fails because that is what the ID of the first node is set to, which obfuscates the problem. There is also nothing to prevent multiple calls to addNode with an argument of NodeID{ 0 }, resulting further nodes added to the graph, with each being assigned a different non-zero ID.

I think an assertion would be helpful to make it immediately obvious what the problem is. Otherwise the code is accepting a user supplied ID, but then silently mutating it. I didn't see any mention in the documentation that Node ID's should be one-indexed either, so a note about that would also be appreciated.

Thanks, Tristan

Operating systems

Windows

What versions of the operating systems?

N/A

Architectures

x86_64

Stacktrace

N/A

Plug-in formats (if applicable)

No response

Plug-in host applications (DAWs) (if applicable)

N/A

Testing on the develop branch

The bug is present on the develop branch

Code of Conduct