Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Night light box #747

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

gpsqueeek
Copy link

Hello,

Here is my second contribution... Not quite the same as the shadebox (with paper cuts, while this box uses wood plates), but I enjoyed when discovering the shadebox too (I was already working on this box at this time)
Please let me know if anything is wrong, so I can improve it.

Thanks for your huge work ! \o/

Have a nice day...

Squeeek

@gpsqueeek
Copy link
Author

It looks like issue #742 is causing a failure here (not sure it is the only one, I am still new to github...)

@florianfesti
Copy link
Owner

OK, let's start with the simpler (not necessarily simple) PR. The result looks pretty nice. The code is very verbose, though. I wonder if it is worth making it more compact.

May the sides, elec compartment and the rails can also be rectangularWalls with custom edges.
Using .polyline() is much more compact than .edge() and .corner(). You can also just put the values into a list and create a reversed copy to draw mirror symmetric parts.
Generally it looks like the draw commands should be reused more aggressively.

Note that all these point are not a strict requirement for this getting merged.

Wrt the CI: Don't worry about it now. It will be easy to fix just before merging.

@gpsqueeek
Copy link
Author

Thanks a lot for the good comment and the advice ! I compacted the code using polyline as much as I could.
Would you have any example of custom edges so I can see how it works ?

Fix hinge hole regarding commit d4b238c
@florianfesti
Copy link
Owner

https://github.com/florianfesti/boxes/blob/master/boxes/generators/bintray.py is one of the easier examples. https://florianfesti.github.io/boxes/html/api_examples.html#bintray describes how it was created from TypeTray. It cheats in not providing a proper Settings object but uses the Boxes object instead - something that could be used here, too.

@gpsqueeek
Copy link
Author

Hi there !
Thanks for pointing these interesting example. I also had a look at the traffic light mentionned in the bin tray doc.
I gave it a try fot the plates slots (so rails and sides), using the same technique as in the bintray to have two variations easily using the self.char to switch between the two cases. It worked fine for the rails, but for the side I stumbled into an issue : currently the back side is not as high as the front side (because of the back of the lid, along the hinge, that went in the way when opening the box), so the rectangularWall would need another specific edge for the back, this seems to make the code even more complicated than it already is. Maybe the way to make the code clearer would be to just create a function to add x slots with this and that depth, thickness and spacing ? Or I could use that custom edge, but not making the side a rectangular wall, which kind of lowers the impact of switching to a custom edge, somehow.
What do you think ?

@florianfesti
Copy link
Owner

You can overwrite the startwidth() and endwidth() methods (and .margin()) to tell pieces like the rectangularWall what kind of corner you need.

@gpsqueeek
Copy link
Author

Thanks for the interesting functions.
For the side it does not seem to me it does not apply really well because the issue I have is that the two opposite edged sides (front and back) do not have the same height ; or I will have to use custom edges on one of these sides too.
For the electronics compartment top it makes sense, but I cannot make it work : I started from the code of the E edge style and adapted it like this :

class OutSetMoreEdge(edges.Edge):
    """Outset (more) edge """
    char = 'B'
    description = "Straight Edge (outset by thickness x 1.5)"
    positive = True
    
    def startwidth(self) -> float:
        return self.thickness*1.5

Unfortunately I get the following error (which I do not get when using the E edge) :

Exception during rendering:
Traceback (most recent call last):
  File "/home/gpsqueeek/boxes/scripts/./boxesserver", line 648, in serve
    box.render()
  File "/home/gpsqueeek/boxes/boxes/scripts/../../boxes/generators/nightlightbox.py", line 345, in render
    self.rectangularWall(self.thickness * (4 if self.BoxStyle == "minimalist" else 8) + self.PlateVisibleWidth + self.Margin, self.BackgroundDepth - self.thickness*5 - self.Margin, "Bfef", move="up", label="elec. comp. debug")
  File "/home/gpsqueeek/boxes/boxes/scripts/../../boxes/__init__.py", line 2437, in rectangularWall
    overallheight = y + edges[0].spacing() + edges[2].spacing()
                        ^^^^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'spacing'
127.0.0.1 - - [20/Jan/2025 15:44:00] "GET /NightLightBox?FingerJoint_style=rectangular&FingerJoint_surroundingspaces=1.0&FingerJoint_bottom_lip=0.0&FingerJoint_edge_width=1.0&FingerJoint_extra_length=0.0&FingerJoint_finger=2.0&FingerJoint_play=0.0&FingerJoint_space=2.0&FingerJoint_width=1.0&Stackable_angle=60&Stackable_bottom_stabilizers=0.0&Stackable_height=2.0&Stackable_holedistance=1.0&Stackable_width=4.0&Hinge_grip_percentage=0&Hinge_outset=0&Hinge_outset=1&Hinge_pinwidth=0.4&Hinge_axle=2.5&Hinge_grip_length=0&Hinge_hingestrength=1&DiffuserPlateTLockScrewDiameter=3.0&DiffuserPlateTLockScrewLength=20.0&DiffuserPlateTLockNutThickness=2.1&DiffuserPlateTLockNutWidth=5.1&BackExtraHoles=R+20+15+11.5+8%0D%0AC+11.58+15+3%0D%0AC+28.42+15+3&BoxStyle=large+face&PlateVisibleWidth=150.0&PlateVisibleHeight=75.0&WoodPlatesCount=3&WoodPlateThickness=5.0&DiffuserPlateThickness=5.0&BackgroundDepth=40.0&InterPlateSpacing=10&Margin=0.5&thickness=5.0&format=svg&tabs=0.0&qr_code=0&debug=0&labels=0&labels=1&reference=100.0&inner_corners=loop&burn=0.1&language=None&render=1 HTTP/1.1" 500 538

I am pretty sure there is something simple missing somewhere regarding that spacing variable (I tried adding it to my class but I still had the same issue).
I probably also could ignore the half thickness difference with the E edge though, but now I want to learn a bit more about what is wrong in my code as it might help me for future developments 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants