sverchok icon indicating copy to clipboard operation
sverchok copied to clipboard

ParticlesMK2 should show meaningful errors for oversized lists

Open EmergencyTemporalShift opened this issue 2 years ago • 6 comments

Problem statement

If you input more vectors than available particles, this node will fail and give the nonspecific error "RuntimeError('internal error setting the array')"

image

Steps to reproduce

  1. Create a list input with a length greater than the number of particles in an emitter
  2. Create a ParticlesMK2 node with the emitter as the object
  3. Wire the output of the list input to the location input of the Particle node

Expected result

A specific error such as "input list extends past the number of available particles"

Actual result

A nonspecific error of "RuntimeError('internal error setting the array')"

Sverchok version

1.1.0 Downloaded sverchok-master.zip from GitHub.

EmergencyTemporalShift avatar Aug 09 '22 03:08 EmergencyTemporalShift

nice catch @EmergencyTemporalShift . A few things to think over:

  • from the nodetree there's no convenient way for a user to gain information about the current particle count of objects
  • would we rather trim the incoming user-array if it exceeds particle count?
  • would we rather automatically extend the user-array with a last value (or some other cycling scheme) if the user-array is shorter than the particle count
  • or do we really want to raise an error.

to keep that code simple on the particles mk2 node, it might be useful to have some way to know the particle counts of objects before we feed data into the particlesmk2 node.

import bpy
obj = bpy.context.active_object
depsgraph = bpy.context.evaluated_depsgraph_get()
particle_systems = obj.evaluated_get(depsgraph).particle_systems
particles = particle_systems[0].particles
print(len(particles))

zeffii avatar Aug 13 '22 12:08 zeffii

you could make a exec node that outputs the number of particles.

import bpy

depsgraph = bpy.context.evaluated_depsgraph_get()
for obj in V1:

    particle_systems = obj.evaluated_get(depsgraph).particle_systems
    particles = particle_systems[0].particles
    append([len(particles)])

image

zeffii avatar Aug 13 '22 17:08 zeffii

hopefully you don't need to use this, but it's an option

zeffii avatar Aug 13 '22 17:08 zeffii

@zeffii interesting you mention under populating the particle list because that has potentially strange behavior as well, it just wraps the input without giving any choice to the user. I feel like more options could be better here.

~~Also, that exec node doesn't seem to be that expensive, the only for loop is looping over objects, not particles, could this become some sort of node output?~~

Oh, you just said that above, never mind.

EmergencyTemporalShift avatar Aug 13 '22 17:08 EmergencyTemporalShift

the issue of extending lists comes down to an opinionated approach. Personally I advocate for doing the list matching in a separate set of nodes, rather than building it into the particle node. Fundamentally because I like to see exactly the ingredients being used inside a node (when it makes sense..).

should obj-particle-count be a node? yeah probably, it could have a variety of modes. Right now we assume the particle system being desired is the first particle system assigned to the object, this is quite primitive support because you might have multiple systems per object, right now ParticlesNode mk2 is oblivious to those possibilities.

there's a bigger discussion to be had, but with particle systems we have been on the fence a little because there was intent by Blender devs to make particles into a nodetree system, much like geometry nodes.

zeffii avatar Aug 13 '22 18:08 zeffii

I think Geometry Nodes is a replacement for particle system, at least some if its functionality. Also there are planes for simulation nodes which also can take some (another) particle system functionality.

Since particle system is going to be deprecated it seems to me it does not worth efforts to commit significant changes to the node, probably fixing error message is enough.

Durman avatar Aug 16 '22 09:08 Durman