-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
[ArchCurtainWall] Add OverrideEdges & ArchSketch Support #13950
[ArchCurtainWall] Add OverrideEdges & ArchSketch Support #13950
Conversation
- Add Overridges property to let user to select particular edge(s) in a Sketch / ArchSketch to use create the shape of the Arch Curtain Wall (instead of using all edges by default). ENHANCEMENT by External 'ArchSketch' Add-on: - GUI 'Edit Curtain Wall' Tool is provided in external Add-on ('SketchArch') to let users to select the edges interactively. - The selection of edges is 'Toponaming-Tolerant' if ArchSketch is used in Base (and SketchArch Add-on is installed). - Warning : Not 'Toponaming-Tolerant' if just Sketch is used. - Property is ignored if Base ArchSketch provided the selected edges. Forum Discussion: - https://forum.freecad.org/viewtopic.php?p=756554#p756554 [ ArchSketch ] - Curtain Wall, Slab, ArchWall etc. on Same ArchSketch
if obj.Base.Shape.Faces: | ||
faces = obj.Base.Shape.Faces | ||
elif obj.Height.Value and obj.VerticalDirection.Length: | ||
ext = FreeCAD.Vector(obj.VerticalDirection) | ||
ext.normalize() | ||
ext = ext.multiply(obj.Height.Value) | ||
faces = [edge.extrude(ext) for edge in obj.Base.Shape.Edges] | ||
# ArchSketch feature : | ||
if hasattr(obj.Base, 'Proxy'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave out all curtain walls whose Base object has a Proxy, but no getCurtainWallBaseShapeEdgesInfo
I thin you need either an else: here, or set a default value for curtainWallBaseShapeEdges
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking :) See if I understand you and below clarify the algorithm :-
-
Line 307 : set curtainWallBaseShapeEdges = None
-
Line 317 : though there is no elif/else after this, the test is in Line 320, 322, 336
-
Line 320 : it test if there is information in 'curtainWallBaseShapeEdges'
-
Line 322 : (elif) if above is not, default would following CurtainWall.OverrideEdges, if the Base is a Sketch
(partially work, i.e. without 'Toponaming-Tolerant' support) -
Line 336 : (else) this catch all, and it revert back to Base's edge - i.e. obj.Base.Shape.Edges
Hope this works, would review if otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right. I didn't look well enough :) Sorry about the noise.
I'll leave this to merge after #13783 okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, feel free to proceed as you plan.
Incidentally, I like the name Arch more than BIM :) And an add-on allow it be updated in a more free timetable not tied to the release cycle of FC itself.
Anyway, will have a look at your big merge to understand your big plan :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, I was myself not sure about which path to take for a long time... But at the end, there is duplication work happening (same things in both Arch and BIM), and also it's time the BIM stuff gets the eyes of everybody (too few contributors when it's an addon on my own github), and also everybody thinks it's a good idea to have a built-in BIM workbench, same as we just did with CAM...
Now that Arch became BIM, isn't it necessary to account for that in this PR? I guess that's what causes the conflicts. |
All the object classes are left in the original Arch files. What has changed is that all the command classes have been moved into the bimcommands subfolder. This PR contains only changes to the Curtain Wall object, so it should be applicable without problems. Only issue I guess, is that the "Arch" folder has been renamed to "BIM" and github doesn't find its way anymore... @paullee could you try this? I think it should work:
|
Thanks very much for the detailed procedure, that's very helpful. I only know very fundamental git command to commit etc. and messed up things very badly a couples of time if did some more than that :D I would see if I manage to follow the steps or 'manually cherry-pick' would be much faster for me : LOL |
I 'finally' re-do, after puzzling a while at Step 1 only :D See : Thanks |
Closing this one as superseded by #14201 |
ENHANCEMENT by External 'ArchSketch' Add-on:
Forum Discussion: