We’ve got a crash that started in 4.14 with one of our sound cues. Here’s the most minimal stripped down version of it I could create:
It was originally 2 banks of random sounds going through a delay and getting concatenated, with the delay there to offset one bank from the other. This delay on this branch isn’t really needed since it’s set to zero, I’m guessing it was just there for consistency. If I remove that delay it doesn’t crash, so I do have a fix, but I figured I’d report it anyway. I’m guessing it showed up because in 4.14 USoundNodeDelay::ParseNodes had a check changed to this: ParseParams.StartTime >= ActualDelay. Previously it was just greater than, so it never would have entered in this case.
I tried actually debugging this, but that code is kind of nuts with the payload macros and such. From what I could understand though the issue is that in USoundNodeConcatenator::ParseNodes it calls ParseNodes then GetNumSounds, and in those two cases different NodeWaveInstanceHash values end up getting passed into USoundNodeRandom. The actual crash is in USoundNodeRandom::GetNumSounds, this line:
if (NodeIndex < ChildNodes.Num() && ChildNodes[NodeIndex])
NodeIndex is typically some giant value that’s out of the array range.
I’m guessing it showed up because in
4.14 USoundNodeDelay::ParseNodes had a check changed to this:
ParseParams.StartTime >= ActualDelay.
Previously it was just greater than,
so it never would have entered in this
case.
Yeah, the fix was to fix up how “StartTime” works with delay nodes – it was basically broken and I found I needed to change that > to a >= (among other things). StartTime is supposed to seek into a sound cue if you imagine a time-line in the sound cue, so it needs to take into account delay nodes, etc.
I was able to repro the crash with your node and my fix was to just add another condition in the delay node conditional to not go into the pass-through parse branch if the the ActualDelay was 0.0 (i.e. the case which should just result in the the delay node passing through).
I tried actually debugging this, but
that code is kind of nuts with the
payload macros and such.
Haha. Yes it is. 
So, digging into this a bit more (and spending some quality time with the payload macro stuff), I realized the bug was actually caused by a subtle issue with how we were retrieving GetNumSounds, which is used by the random node and the concat node. It’s amazing to me this hasn’t crashed yet, but we need to recompute each child node’s child wave instance hash and not forward the parent’s hash down.
What was happening is that the child node hash DID exist in the active sound’s node wave instance hash map but it was the wrong node and a different node type. And in that other node the payload member wasn’t actually initialized. So the result of the macro was that it didn’t think it needed initialization and so it was returning the random uninitialized memory.
This is one of the many ways this system is brittle… I need to spend some time to redesign this system so its less complex and easy to break like this.
So with the fix above (also made in USoundNodeRandom), I don’t get a crash anymore even without my workaround code mentioned in the previous comment.
Thanks a ton for bringing this to my attention!
