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

Openable windows without curtains #13877

Merged
merged 4 commits into from
Nov 6, 2015
Merged

Conversation

OzoneH3
Copy link
Member

@OzoneH3 OzoneH3 commented Nov 2, 2015

It's not pretty, but it gets the job done.

Closes #13858
Closes #7749
Fixes #8165

@@ -461,7 +461,7 @@
[ [ "glass_sheet", 1 ] ]
],
"pre_terrain" : "t_window_empty",
"post_terrain" : "t_window"
"post_terrain" : "t_window_no_curtains"
},{
"type" : "construction",
"description" : "Build Window", "//": "Step 3: window with curtains",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this one should even stay, you won't be able to build t_window just find them in stores.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution to the store window being special!

@BevapDin
Copy link
Contributor

BevapDin commented Nov 3, 2015

There are a few mapgens of houses that should probably use the new windows.

data/json/mapgen/house_quiverfull.json comes to mind, it currently has only standard "t_window" (without curtains), which looks a bit strange for a private building.

@OzoneH3
Copy link
Member Author

OzoneH3 commented Nov 3, 2015

I can fix those houses if someone points me to them but otherwise "fixing" houses is not really in the scope of this pr.

As for house_quiverfull should it be t_window_domestic or was it intended for the windows to have no curtains and use t_window_no_curtains?

"difficulty" : 2,
"time" : 30,
"qualities": [ [
{ "id": "SAW_W", "level": 1 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably have HAMMERING 2 too, to drive the nails in.

@Coolthulhu Coolthulhu self-assigned this Nov 4, 2015
@Coolthulhu
Copy link
Contributor

This new window needs some different name and preferably either color or glyph too.
Currently it's not possible to tell which window has curtains without walking up to it first.

@Coolthulhu Coolthulhu removed their assignment Nov 4, 2015
@BevapDin
Copy link
Contributor

BevapDin commented Nov 5, 2015

As for house_quiverfull should it be t_window_domestic or was it intended for the windows to have no curtains and use t_window_no_curtains?

I though it was intended to be windows without curtains. A private house that has windows that can not be opened looks strange to me, but that may be intended. @jokermatt999 (the creator) should know.

@jokermatt999
Copy link
Contributor

Nope, just a mistake. I've been meaning to get back into developing, so
I'll whip up a fix later tonight. I've got a few buildings I wanted to
tweak actually.
On Nov 5, 2015 8:38 AM, "BevapDin" notifications@github.com wrote:

As for house_quiverfull should it be t_window_domestic or was it intended
for the windows to have no curtains and use t_window_no_curtains?

I though it was intended to be windows without curtains. A private house
that has windows that can not be opened looks strange to me, but that may
be intended. @jokermatt999 https://github.com/jokermatt999 (the
creator) should know.


Reply to this email directly or view it on GitHub
#13877 (comment)
.

@OzoneH3
Copy link
Member Author

OzoneH3 commented Nov 5, 2015

So, I'll leave the house_quiverfull to jokermatt999 then.

Recolored domestic windows:
w/ curtains: c_ltgrey
w/o curtains: c_white
Left the commercial ones ltcyan and all taped variants dkgrey.

@Coolthulhu Coolthulhu self-assigned this Nov 6, 2015
Coolthulhu added a commit that referenced this pull request Nov 6, 2015
Openable windows without curtains
@Coolthulhu Coolthulhu merged commit 126e15c into CleverRaven:master Nov 6, 2015
@MisterFelixFox
Copy link
Contributor

I never thought they'd get used!

@OzoneH3 OzoneH3 deleted the no_curtains branch November 10, 2015 20:53
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.

None yet

6 participants