Changeset View
Standalone View
source/blender/blenkernel/intern/particle.c
| Show First 20 Lines • Show All 3,961 Lines • ▼ Show 20 Lines | |||||
| ModifierData *object_copy_particle_system(Main *bmain, | ModifierData *object_copy_particle_system(Main *bmain, | ||||
| Scene *scene, | Scene *scene, | ||||
| Object *ob, | Object *ob, | ||||
| const ParticleSystem *psys_orig) | const ParticleSystem *psys_orig) | ||||
| { | { | ||||
| return object_add_or_copy_particle_system(bmain, scene, ob, NULL, psys_orig); | return object_add_or_copy_particle_system(bmain, scene, ob, NULL, psys_orig); | ||||
| } | } | ||||
| void object_remove_particle_system(Main *bmain, Scene *UNUSED(scene), Object *ob) | void object_remove_particle_system(Main *bmain, | ||||
| Scene *UNUSED(scene), | |||||
| Object *ob, | |||||
| ParticleSystem *psys) | |||||
campbellbarton: Might it make more sense to take a `ParticleSystem *psys` argument that's never NULL?
This way… | |||||
Done Inline ActionsI haven't used that approach because:
I agree, that passing the particle system directly would simplify the code and the doc-string should be added for the current approach. I'm fine with either approach, if you or @Bastien Montagne (mont29) have a strong preference, then I'll implement that approach. rjg: I haven't used that approach because:
- passing the modifier removes the need to iterate… | |||||
Done Inline ActionsNo worries, also not _that_ opinionated in this case, trust @Bastien Montagne (mont29)'s judgement. campbellbarton: No worries, also not _that_ opinionated in this case, trust @mont29's judgement. | |||||
Done Inline ActionsIn my comment in chat I actually suggested to pass the psys yes, so would rather do that here too. Was only suggesting to make it optional, byut given the number of calls to that function, would not mind making psys parameter mandatory. Don't think calls to psys_get_modifier() are a real issue in practice (this code is called from operators, not from internal low-level areas executed thousands of times per seconds ;) ). In fact think skipping them in this case is wrong, even if you pass in a particle modifier, you still want to unlink references to its particle system in other existing physics modifiers anyway. So the only call you'd save would be the one to psys_get_modifier, not worth it imho. mont29: In my comment in chat I actually suggested to pass the `psys` yes, so would rather do that here… | |||||
Done Inline ActionsRight, sorry for the misunderstanding. I will fixes this during the weekend. rjg: Right, sorry for the misunderstanding. I will fixes this during the weekend. | |||||
| { | { | ||||
| ParticleSystem *psys = psys_get_current(ob); | if (!ob || !psys) { | ||||
| ParticleSystemModifierData *psmd; | |||||
| ModifierData *md; | |||||
| if (!psys) { | |||||
| return; | return; | ||||
| } | } | ||||
| ParticleSystemModifierData *psmd; | |||||
| ModifierData *md; | |||||
| /* Clear particle system in fluid modifier. */ | /* Clear particle system in fluid modifier. */ | ||||
| if ((md = BKE_modifiers_findby_type(ob, eModifierType_Fluid))) { | if ((md = BKE_modifiers_findby_type(ob, eModifierType_Fluid))) { | ||||
| FluidModifierData *fmd = (FluidModifierData *)md; | FluidModifierData *fmd = (FluidModifierData *)md; | ||||
| /* Clear particle system pointer in flow settings. */ | /* Clear particle system pointer in flow settings. */ | ||||
| if ((fmd->type == MOD_FLUID_TYPE_FLOW) && fmd->flow && fmd->flow->psys) { | if ((fmd->type == MOD_FLUID_TYPE_FLOW) && fmd->flow && fmd->flow->psys) { | ||||
| if (fmd->flow->psys == psys) { | if (fmd->flow->psys == psys) { | ||||
| fmd->flow->psys = NULL; | fmd->flow->psys = NULL; | ||||
| ▲ Show 20 Lines • Show All 1,418 Lines • Show Last 20 Lines | |||||
Might it make more sense to take a ParticleSystem *psys argument that's never NULL?
This way it's always clear which particle system is being removed and the caller is always passing in valid data.
This seems a little simpler then passing in a modifier which can be null.
If using a modifier argument is preferable, there should be a doc-string here that notes it can be NULL which uses a the active particle system as a fallback.