-
Notifications
You must be signed in to change notification settings - Fork 41
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
Support building types #397
Conversation
Depending on what type of building is selected, space types will be pre-generated into the list of space types available. User-defined space types will always appear first across all building types.
* @param {boolean} wholeBuilding Whether or not the space types are generated for the whole building | ||
* @returns {{[key: string]: string}} | ||
*/ | ||
spaceTypesForBuilding(building_type, template, wholeBuilding = false) { |
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 logic is pulled from https://github.com/NREL/openstudio-extension-gem/blob/develop/lib/openstudio/extension/core/os_lib_model_generation.rb#L404-L696. If I know for sure that we don't need the additional data there, I can simplify the logic here and cut down on some of the redundancy.
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.
it looks like the keys and values are always identical. Maybe a Set or Array would be a better choice of data structure?
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.
The UI expects to be returned an object with id - name pairings; however, I forgot to update the names to be user friendly, though, so I'll go through and update them.
Simplify logic in helper and add some docs
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.
Great work! Also, thank you for fixing up the inconsistent white spacing. It's been driving me nuts. Left one possible performance comment otherwise looks good to me.
const space = payload.space; | ||
|
||
if (payload.building_type_id && space.building_unit_id) { | ||
state.stories.forEach((story) => { |
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'm not sure if a map would be more performant here per the comment you left on my last PR. #399 (comment)
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.
Yeah, I can change that
* @param {boolean} wholeBuilding Whether or not the space types are generated for the whole building | ||
* @returns {{[key: string]: string}} | ||
*/ | ||
spaceTypesForBuilding(building_type, template, wholeBuilding = false) { |
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.
it looks like the keys and values are always identical. Maybe a Set or Array would be a better choice of data structure?
"below_floor_plenum_height":0,"floor_to_ceiling_height":0,"multiplier":0,"spaces":[{"id":"13","name":"Living 3","color":"#8BC600","handle":null,"face_id":null,"daylighting_controls":[],"building_unit_id":null,"thermal_zone_id":null,"space_type_id":"17","construction_set_id":null},{"id":"16", | ||
"name":"Attic 3","color":"#557a00","handle":null,"face_id":null,"daylighting_controls":[],"building_unit_id":null,"thermal_zone_id":null,"space_type_id":"21","construction_set_id":null}],"windows":[],"shading":[],"images":[],"geometry_id":"12"}],"library":{"building_units":[],"thermal_zones":[ | ||
{"id":"22","color":"#007373","name":"Thermal Zone 1"},{"id":"23","color":"#000000","name":"Thermal Zone 2"}],"space_types":[{"id":"17","color":"#007373","name":"Living","type":"space_types"},{"id":"18","color":"#000000","name":"Basement","type":"space_types"}, | ||
{"id":"19","color":"#004749","name":"Crawlspace","type":"space_types"},{"id":"20","color":"#009196","name":"Garage","type":"space_types"},{"id":"21","color":"#8BC600","name":"Attic","type":"space_types"}],"construction_sets":[],"windows":[],"daylighting_controls":[]}}`); |
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.
🔥
Resolves #363
Adds in a select field for building types in the assignment section of the UI. There is a default list of building types provided for the project, but the user can modify the list however they want. There is also a freeform text input for template that has been added. The
building_unit
field of a space is used to bind spaces into the same building. Modifying the building type of a space that shares the same building unit as another space will change the building type of that space as well; conversely, modifying the building unit of a space will change that space's building type to match the building type of all spaces in that building unit.In addition to this, the space types select now reacts to changes in building type and template to pre-generate a list of space types for recognized combinations of building type and template. These space types are always appended after user-defined space types.
The list of space types was pulled from here. I'm not sure what to do with the extra information stored there - I assume we can just drop it for now, but would appreciate clarification. Once I have that I can clean up the
helpers.js
file all that code got dumped into.