Page MenuHome
Differential D302 Diff 1030 source/blender/editors/space_view3d/view3d_snap.c

Changeset View

Standalone View

View Options

source/blender/editors/space_view3d/view3d_snap.c

Context not available.
return true; return true;
} }
campbellbartonAuthorUnsubmitted
Not Done
Inline Actions

A bit misleading since this is a view3d operator, but it doesnt really require a 3d view, see: object_warp_verts_poll, this poll function could be made generic and apart of ED_transverts

campbellbarton: A bit misleading since this is a view3d operator, but it doesnt really require a 3d view, see…
enum{
RND_X = (1 << 0),
aligorithUnsubmitted
Not Done
Inline Actions

It would be better to do:
RND_X = (1 << 0),
RND_Y = (1 << 1),
...
RND_Z = (1 << 2),

aligorith: It would be better to do: RND_X = (1 << 0), RND_Y = (1 << 1), ... RND_Z = (1 << 2),
RND_Y = (1 << 1),
RND_Z = (1 << 2)
};
campbellbartonAuthorUnsubmitted
Not Done
Inline Actions

Why define these? just (or) | any of the 3 flags. This is what enum flags are for.

campbellbarton: Why define these? just (or) `|` any of the 3 flags. This is what enum flags are for.
static EnumPropertyItem rnd_object_axes[] = {
{RND_X, "X_AXIS", 0, "Randomize X", ""},
{RND_Y, "Y_AXIS", 0, "Randomize Y", ""},
{RND_Z, "Z_AXIS", 0, "Randomize Z", ""},
{RND_X | RND_Y, NULL, 0, NULL, NULL},
campbellbartonAuthorUnsubmitted
Not Done
Inline Actions

Looks like these do nothing.

campbellbarton: Looks like these do nothing.
{RND_X | RND_Z, NULL, 0, NULL, NULL},
{RND_Y | RND_Z, NULL, 0, NULL, NULL},
{RND_X | RND_Y | RND_Z, NULL, 0, NULL, NULL},
aligorithUnsubmitted
Not Done
Inline Actions

Instead of repeating the values for each of the axis combinations here, use the enum defines you've made above. For example, this line becomes:
{RND_X, "X_AXIS", 0, "Randomize X", ""},

Also, if you intend for the other options to be available, you'll need to define these here too.

aligorith: Instead of repeating the values for each of the axis combinations here, use the enum defines…
{0, NULL, 0, NULL, NULL}
};
static int ED_VIEW3D_randomize_verts_exec(bContext *C, wmOperator *op)
{
float amp;
int freq, flag;
bool x = false, y = false, z = false;
Object *obedit = CTX_data_edit_object(C);
amp = RNA_float_get(op->ptr, "amplitude");
freq = RNA_int_get(op->ptr, "step_size");
flag = RNA_enum_get(op->ptr, "axes");
x = (flag & RND_X) != 0;
y = (flag & RND_Y) != 0;
campbellbartonAuthorUnsubmitted
Not Done
Inline Actions

This could be an enum-flag, so you could select X and Y, or Y and Z for example.

See: RNA_def_enum_flag use in editmesh_tools.c. Then ED_transverts_randomize would take one axis arg.

campbellbarton: This could be an enum-flag, so you could select X and Y, or Y and Z for example. See…
z = (flag & RND_Z) != 0;
aligorithUnsubmitted
Not Done
Inline Actions

This whole block of if's can be simplified by simply doing something like:
x = (flag & RND_X) != 0;
y = (flag & RND_Y) != 0;
z = (flag & RND_Z) != 0;

aligorith: This whole block of if's can be simplified by simply doing something like: x = (flag & RND_X) !
ED_transverts_randomize(obedit, x, y, z, amp, freq);
WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, obedit);
return OPERATOR_FINISHED;
}
void VIEW3D_OT_randomize_verts(wmOperatorType *ot)
{
ot->name = "Randomize";
ot->description = "Apply random offset to vertex locations";
ot->idname = "VIEW3D_OT_randomize_verts";
ot->poll = ED_transverts_obedit_poll;
ot->exec = ED_VIEW3D_randomize_verts_exec;
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
aligorithUnsubmitted
Not Done
Inline Actions

Perhaps "Apply random offsets to vertex locations" might be more descriptive?

aligorith: Perhaps "Apply random offsets to vertex locations" might be more descriptive?
ot->prop = RNA_def_enum_flag(ot->srna, "axes", rnd_object_axes, RND_X | RND_Y | RND_Z, "Axes", "Which axis to affect: X, Y, Z");
RNA_def_float(ot->srna, "amplitude", 0.1f, 0.0f, 1.0f, "Amplitude", "", 0.0f, 1.0f);
campbellbartonAuthorUnsubmitted
Not Done
Inline Actions

use lowecase naming for properties.

campbellbarton: use lowecase naming for properties.
RNA_def_int(ot->srna, "step_size", 1, 1, 10000, "Step Size", "Effect every nth vertex", 1, 10000);
}
Context not available.
aligorithUnsubmitted
Not Done
Inline Actions

It's better to fully spell out the names of the identifiers here instead of abbreviating them. So, "amplitude" instead of "amp", and the same for frequency I guess.

aligorith: It's better to fully spell out the names of the identifiers here instead of abbreviating them.
aligorithUnsubmitted
Not Done
Inline Actions

Typo - "Effect ever nth vertex" ;)

Personally, I'd probably name this "step_size" or something similar instead. Judging by the name, I'd have thought that this controls the frequency of the underlying noise function instead.

aligorith: Typo - "Effect **ever** nth vertex" ;) Personally, I'd probably name this "step_size" or…