Use Custom Blocks in Foundation Kiva #3567
Conversation
joseph-robertson
added
the
Pull Request - Ready for CI
|
I pulled the windows installer from ci and tested it against our residential workflow. It works:
|
|
@joseph-robertson Can you send me the installer? I'll test it on a branch of one of my repos that is going to use it. |
| @@ -6339,21 +6340,24 @@ OS:Foundation:Kiva, | |||
| \type real | |||
| \minimum> 0.0 | |||
| \default 0.3 | |||
| N10, \field Number of Custom Blocks | |||
| \minimum 0.0 | |||
There was a problem hiding this comment.
Shouldn't this be formatted as an integer (no decimal point), also can be autocalculated. See OS:Surface Number of Vertices
There was a problem hiding this comment.
I was using OS:Generator:FuelSupply as a guide. It has \minimum=0.0 and \maximum=12.0. But yes I will take a look at OS:Surface Number of Vertices.
There was a problem hiding this comment.
Decided to remove this field from the idd.
| return numExtensibleGroups(); | ||
| } | ||
|
|
||
| bool FoundationKiva_Impl::setNumberofCustomBlocks(unsigned int numberofCustomBlocks) { |
There was a problem hiding this comment.
I would remove this method
| return result; | ||
| } | ||
|
|
||
| void FoundationKiva_Impl::resetNumberofCustomBlocks() { |
There was a problem hiding this comment.
I would remove this method
| return addCustomBlock(customBlock); | ||
| } | ||
|
|
||
| bool FoundationKiva_Impl::removeCustomBlock(unsigned groupIndex) { |
There was a problem hiding this comment.
I have tended to not include this method, it is hard to use. I think its easier to just clear all the blocks and then set all of them with a vector. If you keep this method it should have a lot of tests
There was a problem hiding this comment.
Decided to keep this method and have a lot of tests.
|
|
||
| //should fail since only 10 allowed | ||
| ASSERT_FALSE(kiva.addCustomBlock(material, 1, 1, 1)); | ||
| kiva.removeCustomBlock(9); |
There was a problem hiding this comment.
Here you would need to check that removing block i correctly moves other blocks around, that you get the right blocks back from customBlocks, etc.
There was a problem hiding this comment.
Added tests for this.
| kiva.removeCustomBlock(0); | ||
| EXPECT_EQ(0, kiva.numberofCustomBlocks().get()); | ||
|
|
||
| CustomBlock customBlock(material, 0.5, 1, -1); |
There was a problem hiding this comment.
Need more tests for customBlocks and setCustomBlocks
There was a problem hiding this comment.
Added addCustomBlocks(...) method and tests.
|
Thanks for making these updates @joseph-robertson ! |
Design doc: https://docs.google.com/document/d/1zu-kOTe3q22X2ukZKKg0vn7_2fvhZpakXeHZ05y1-LA/edit#