Page MenuHome

Multi-Object-Editing : Support for MESH_OT_rip
ClosedPublic

Authored by Miguel Pozo (pragma37) on May 9 2018, 1:16 AM.

Details

Summary

I have changed the OPERATOR_CANCELLED returns to simple continues inside the loop, just as in the examples.
Is this the final design ??? Feels weird.

Diff Detail

Repository
rB Blender

Event Timeline

I tried the same edits before some minutes.
I was not sure about changing the invoke functions prototypes to take the Object as argument.
In addition i had a dilemma about if BKE_report should be thrown or not when we have a face selection in one object an an edge in others..
final decide to abort it...

edbm_rip_invoke_vert and edbm_rip_invoke_edge are called only once in the whole codebase and they are not even declared in any header, so I assume it's fine to change the function signature.

Regarding the reports and managing failures I feel the same and had a simmilar problem on other operator. That's why I'm asking for feedback, since even the examples ignores the issue.

i have no idea, i joined today after the call ...
i do not know anything about how the blender code is structured except what i have seen in the last hours...

Dalai Felinto (dfelinto) requested changes to this revision.May 9 2018, 10:26 AM

The patch works, however:

No need to run RNA_boolean_get(op->ptr, "use_fill"); for every object either. Better to run it once and pass as an argument.

I had a dilemma about if BKE_report should be thrown or not when we have a face selection in one object an an edge in others.

You shouldn't print the error message for every object. In fact it should only report if it fails for the same reason for all of them. See for instance what I did on rB66ffbf9b7d8. The idea is that for a single object it should work just as in 2.7x (as you did), but when using multiple objects, we should only report errors if that prevented the operator to run entirely.

This revision now requires changes to proceed.May 9 2018, 10:26 AM

Updated with the changes requested by @Dalai Felinto (dfelinto).

I simplified a bit the error reports since most of them were due to invalid internal state.
I made another version where the edbm_rip_invoke functions returned a different enum value for each error, but it felt unnecessarily convoluted.

This revision was not accepted when it landed; it landed in state Needs Review.May 11 2018, 11:00 AM
This revision was automatically updated to reflect the committed changes.