kennetek / gridfinity-rebuilt-openscad

A ground-up rebuild of the stock gridfinity bins in OpenSCAD
Other
1.13k stars 164 forks source link

Snaps - allow for snaps and/or dovetails #184

Open yuval-k opened 2 months ago

yuval-k commented 2 months ago

Each face can choose a F or M connector. you can also print a snap for F-F connections.

Example: gridfinity-rebuilt-baseplate

related: https://github.com/kennetek/gridfinity-rebuilt-openscad/issues/67

EmperorArthur commented 1 month ago

Neat!

I do have a few suggestions as someone who's recently been in that code pretty heavily. See #187 which is not quite compatible with your PR.

First, is it really worth it to allow choosing each side? Our current code is all or nothing for everything else, and it seems like more trouble than it's worth. I do like the approach of combining them all in a list, and I am doing something similar in the hole cutter code myself.

Second, documentation. I know it's not sexy, but commenting functions and any non-obvious parameters helps us all when reading later. It's saved my but more times than I'd like to admit when I ask myself WTF I was thinking 6 months down the line.

Third, it's probably worth it to replace the "print_snap" with a separate file that calls pattern_linear(...){snap()}. You could move snap into "gridfinity-rebuilt-utility.scad", but that makes having the function and parameters documented even more important.

Fourth, and this is minor, adder_snaps and cutter_snaps don't seem to really do anything useful compared to just calling snaps_on_sides directly. I'd agree with the idea of having cutter and adder interfaces to swap out the holes with snaps, but that's not really something OpenSCAD is designed to do.

I'd also ask if the unit tests ran, but those are also in #187 and are far from complete. Instead would you mind including pictures of what some of the other modes, like with holes, looks like.