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

[CR] Create Wall wiring #60671

Merged
merged 2 commits into from
Sep 5, 2022

Conversation

Maleclypse
Copy link
Member

@Maleclypse Maleclypse commented Sep 2, 2022

Summary

Content "Create Wall wiring for walls without hidden wiring"

Purpose of change

Adds a construction to place new wiring for walls that lack wiring.

Describe the solution

Adds the needed construction. I probably need some additional things in here but this is basics and I hope it receives comments on how to improve it. I believe it needs some kind of terrain check so it only goes on walls but I have to walk away from this for a minute.

Describe alternatives you've considered

Waiting on other people.

Testing

Tests so far have all worked.

Additional context

Reveal wiring pre-change was performed on the upper walls and place wiring was performed on the bottom walls post change.

You can use both reveal wiring and place wiring on walls that remain pre-wired.

Reveal wiring top place wiring bottom

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Sep 2, 2022
@Maleclypse Maleclypse changed the title Create Wall wiring [CR] Create Wall wiring Sep 2, 2022
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Sep 2, 2022
@PatrikLundell
Copy link
Contributor

Some rambling thoughts:

Hm, the implementation implies it's wiring just taped onto a surface, and such an installation doesn't care much about what that surface is, so it ought to be possible to install it on anything solid, from rock walls to trees, to floors. Given the usage of duct tape it probably shouldn't work on soil or dirt/chip floors (and probably not rammed earth and natural earth walls either, but that can be hand waved away by some implicit usage of something else to fasten the wiring with).

It shouldn't be possible to install it in the air or on/in liquids.

Installation on floors/ceilings is currently problematic because there's no way to disable collisions with vehicles, so installing wiring through a room would nonsensically block the ability to pull a shopping cart through it, so despite it being logical to be able to attach wiring to a floor, this restriction might result in to conclusion that you'd want to restrict the installation terrain to the ones that cannot be traversed.

I'd probably let the installation take some time, e.g. 5 minutes.

Aren't tools exempt from being consumed, so they wouldn't need negative modifiers (asking because I think it's looks wrong, but I'm not sure it is)?

Aren't there tool qualities that can be used rather than two explicit tools, or at least tool categories so there's some leeway in what tools you can use (again, I'm not confident in my understanding of how this works)?

@Maleclypse
Copy link
Member Author

Made most of the changes suggested. The tools I left negative because it's shown that way in other locations and those two specific tools because you we currently list pliers as providing wrench 1, and the tools with wrench 2 would be inappropriate for use in this type of project and you can't make something require wrench 1 but can't be fulfilled by wrench 2. Anyway adjusting pliers to be something other than wrench 1 is out of scope for this PR.

@PatrikLundell
Copy link
Contributor

Won't removal of pre wired walls have an unpleasant effect on existing saves that rely on them?

Also, won't this remove wiring from most pre cataclysmic buildings, including those that definitely should be wired (although you could argue that the wiring is useless until disconnected from the broken grid)?

I'm leaning toward retaining the wired walls and define new set of similar unwired walls that would be the types survivors could build (possibly adding construction of the wired varieties by adding construction materials for it).

@Maleclypse
Copy link
Member Author

Won't removal of pre wired walls have an unpleasant effect on existing saves that rely on them?

Also, won't this remove wiring from most pre cataclysmic buildings, including those that definitely should be wired (although you could argue that the wiring is useless until disconnected from the broken grid)?

I'm leaning toward retaining the wired walls and define new set of similar unwired walls that would be the types survivors could build (possibly adding construction of the wired varieties by adding construction materials for it).

So the ones I removed are the ones that you wouldn’t be able to just reveal the wiring in. Brick, concrete, log, and plank. Plank seems to only be in barns. And the others would have pre built conduit in them for electrical it if they had any at all.

I’ll have to check and see if it has any impact on any pre-existing deployments but my assumption was that it’s only checked when first built. But you are right I should test that assumption thoroughly.

@Maleclypse Maleclypse marked this pull request as draft September 4, 2022 13:10
@PatrikLundell
Copy link
Contributor

Even if it's tested only on deployment, it's going to be really confusing to find that you suddenly can't add any new appliances to the building you've already added some appliances to, if that happens to be the case. I don't have a problem with it happening if the player gets an out of game explanation, as it shouldn't have been possible in the first place.

@Maleclypse
Copy link
Member Author

Maleclypse commented Sep 4, 2022

Tested and confirmed that this will not impact any pre-made wiring the character has made before this is merged, if it's merged in current state.

@Drew4484
Copy link
Contributor

Drew4484 commented Sep 4, 2022

Would it be possible to add wiring through/over doorframes?

@Maleclypse
Copy link
Member Author

Would it be possible to add wiring through/over doorframes?

So it would have to be added as a terrain and use c++ because vehicles always block movement of hauling and grabbed items. So it would stop being a doorframe and become a vaguely open window effectively.

@Maleclypse
Copy link
Member Author

Even if it's tested only on deployment, it's going to be really confusing to find that you suddenly can't add any new appliances to the building you've already added some appliances to, if that happens to be the case. I don't have a problem with it happening if the player gets an out of game explanation, as it shouldn't have been possible in the first place.

How would you suggest the out of game explanation? Like a reddit post once it's merged?

@Maleclypse Maleclypse marked this pull request as ready for review September 4, 2022 16:40
@PatrikLundell
Copy link
Contributor

Even if it's tested only on deployment, it's going to be really confusing to find that you suddenly can't add any new appliances to the building you've already added some appliances to, if that happens to be the case. I don't have a problem with it happening if the player gets an out of game explanation, as it shouldn't have been possible in the first place.

How would you suggest the out of game explanation? Like a reddit post once it's merged?
I didn't mean outside of the game, but breaking the 4:th wall, i.e. a message to the player rather than the character.
I don't have a good idea how, though. Ideally something on loading the save, returning to a base camp, or examining the bulletin board on a base camp.

You're right about the door frame, unfortunately. I should have realized that it wouldn't work.

@bombasticSlacks bombasticSlacks merged commit 8873158 into CleverRaven:master Sep 5, 2022
@irwiss irwiss mentioned this pull request Sep 5, 2022
@Maleclypse Maleclypse deleted the Create-Wall-wiring branch September 6, 2022 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Crafting / Construction / Recipes Includes: Uncrafting / Disassembling [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants