Fix #98583: Appending a particle system pulls in an unused collection #104814

Philipp Oeser · 2023-02-16T12:10:50+01:00

Philipp Oeser commented

2023-02-16 12:10:50 +01:00

When setting a ParticleSettings->instance_collection but having
ParticleSettings->render_type something other than PART_DRAW_GR, the
collection would still be pulled in when appending. Same thing for
ParticleSettings->instance_object and PART_DRAW_OB.

This seems unnecessary, and since we do similar - conditional -
expansions with BLO_expand, this patch now only pulls in the object/
collection when actually used.

When setting a `ParticleSettings->instance_collection` but having `ParticleSettings->render_type` something other than `PART_DRAW_GR`, the collection would still be pulled in when appending. Same thing for `ParticleSettings->instance_object` and `PART_DRAW_OB`. This seems unnecessary, and since we do similar - conditional - expansions with `BLO_expand`, this patch now only pulls in the object/ collection when actually used.

Philipp Oeser added 1 commit 2023-02-16 12:11:04 +01:00

55b3352ba1 Fix #98583 : Appending a particle system pulls in an unused collection

When setting a `ParticleSettings->instance_collection` but having
`ParticleSettings->render_type` something other than `PART_DRAW_GR`, the
collection would still be pulled in when appending. Same thing for
`ParticleSettings->instance_object` and `PART_DRAW_OB`.

This seems unnecessary, and since we do similar - conditional -
expansions with `BLO_expand`, this patch now only pulls in the object/
collection when actually used.

Bastien Montagne was assigned by Philipp Oeser

2023-02-16 12:11:36 +01:00

Bastien Montagne was unassigned by Philipp Oeser

2023-02-16 12:12:00 +01:00

Philipp Oeser requested review from Bastien Montagne 2023-02-16 12:12:07 +01:00

Philipp Oeser added this to the Core project 2023-02-16 12:12:28 +01:00

Bastien Montagne commented

2023-02-16 12:45:11 +01:00

I would not accept this change at all. If you follow that logic, then you can also not link the force collection if its not needed etc.

Bastien Montagne closed this pull request

2023-02-16 12:45:11 +01:00

Philipp Oeser commented

2023-02-16 13:15:11 +01:00

Starting to implicitly filter out what data we import based on what is currently exposed to the user is a hard no-go.

If you follow that logic, then you can also not link the force collection if its not needed etc

OK, not sure why this be such a hard no-go, not pulling stuff in that is definitely not needed does not seem very alien to me? From the user perspective, this is not much different to not saving orphans.

But yeah, having a hard rule is good, maybe another much more consistent approach would be to actually remove the collection/object user already when switching the source (basically setting it to NULL)? -- or leave everything as-is and just make people aware they have to do this "by hand" :)

Anyways, thx for the feedback/review!

> Starting to implicitly filter out what data we import based on what is currently exposed to the user is a hard no-go. > If you follow that logic, then you can also not link the force collection if its not needed etc OK, not sure why this be such a hard no-go, not pulling stuff in that is definitely not needed does not seem very alien to me? From the user perspective, this is not much different to not saving orphans. But yeah, having a hard rule is good, maybe another much more consistent approach would be to actually remove the collection/object user already when switching the source (basically setting it to NULL)? -- or leave everything as-is and just make people aware they have to do this "by hand" :) Anyways, thx for the feedback/review!

Bastien Montagne commented

2023-02-16 14:06:28 +01:00

If we wanted that behavior, indeed non-currently-used pointers should be cleared when the source is switched. That way info would not be stored in file etc. This is ore of a UI/UX/product topic though.

But as long as the reference to the collection is stored in the file, it should be pulled in when linking or appending (or just reading for what matters).

If we wanted that behavior, indeed non-currently-used pointers should be cleared when the source is switched. That way info would not be stored in file etc. This is ore of a UI/UX/product topic though. But as long as the reference to the collection is stored in the file, it should be pulled in when linking or appending (or just reading for what matters).

Bastien Montagne removed this from the Core project 2023-07-03 12:49:08 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #98583: Appending a particle system pulls in an unused collection #104814

Pull request closed