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

Un-Swappable Storage Batteries #34741

Closed
LetterShapedGlyphs opened this issue Oct 14, 2019 · 12 comments · Fixed by #34850
Closed

Un-Swappable Storage Batteries #34741

LetterShapedGlyphs opened this issue Oct 14, 2019 · 12 comments · Fixed by #34850
Labels
<Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling (S2 - Confirmed) Bug that's been confirmed to exist Vehicles Vehicles, parts, mechanics & interactions

Comments

@LetterShapedGlyphs
Copy link
Contributor

LetterShapedGlyphs commented Oct 14, 2019

Describe the bug

Storage batteries cannot be installed into their case without using a welder.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Create a vehicle with a swappable storage battery case.
  2. Have a nearby storage battery
  3. Attempt to install the battery without a welder.

Expected behavior

The storage battery can be installed provided you have the strength/lifting to do so.

Versions and configuration

  • OS: Windows 10
  • Game Version: 0.D-8505-gf66b63b
  • Graphics version: Tiles
  • Mods loaded: Alternative Map Key, No Antique Firearms, No Survivor Armor, Sleep Deprivation, Stats Through Skills

Additional context

A change from six months ago #29595 made storage batteries use non-legacy requirements. As the code currently stands, this prevents the removable version from being installed without a welder. The "using" array remains present in the removable copy. I have a current naive solution working which uses the fact that a single string "using" will overwrite the array of the base object, but will submit a PR later this evening with a hopefully more robust solution.

@esotericist
Copy link
Contributor

esotericist commented Oct 14, 2019

First off: Mods are never N/A when reporting a bug like this. Please include that information. As of experimental versions a while ago, game helpfully has a tool to populate the information needed for bug reports, when you hit esc, in the debug info.

Second: that change from six months ago cannot possibly be responsible for this problem, seeing as how I was installing swappable storage batteries without welding for months afterwards.

@esotericist esotericist added (S2 - Confirmed) Bug that's been confirmed to exist <Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Vehicles Vehicles, parts, mechanics & interactions labels Oct 14, 2019
@LetterShapedGlyphs
Copy link
Contributor Author

I have added the mods I am using. As you can see, they clearly have zero impact on the relevant section of the game and are, as such, not applicable to this bug.

@esotericist
Copy link
Contributor

I've already confirmed the bug without mods, I'm just stating that as a matter of policy for bug reports of this type, N/A is incorrect; it's always possible one of them does have impact and you just didn't realize it.

@LetterShapedGlyphs
Copy link
Contributor Author

So in the future when I duplicate the bug in my modless testbed should I put "None" instead of N/A?

@esotericist
Copy link
Contributor

So in the future when I duplicate the bug in my modless testbed should I put "None" instead of N/A?

Yes, but I would request in such a case you explicitly state you tested it without mods; we've had instances in the past with users reporting "none" when they had an arbitrary mod they just assumed couldn't possibly be the problem and thus not worth mentioning (even though it turned out to be the problem). It's best to not have any ambiguity.

@esotericist
Copy link
Contributor

I've narrowed the cause down to #34455 by @jbytheway.

I'm not sure what the correct fix is here, hopefully he can shed some light since he's been putting a lot of work into json parsing cleanup lately.

@LetterShapedGlyphs
Copy link
Contributor Author

I have a workaround fix that "solves the problem" by requiring a screwdriver, but I have a better solution in mind that just requires me to go through the vehicle parts json files.

@kevingranade
Copy link
Member

Could you outline that solution? I'm sceptical of a solution that requires you to edit anything ut the involved code or parts.

@LetterShapedGlyphs
Copy link
Contributor Author

I'm not sure where you got the idea that vehicleparts json files aren't involved as they are the reason that jbytheway made the changes to the "using" code that he did. By figuring out what needs to be changed on both ends, I can hopefully either reverse his change, or possibly render the entire legacy section of veh_type.cpp obsolete by updating the json.

@jbytheway
Copy link
Contributor

I assume it must have been the change to veh_type.cpp in #34455 that caused this, but I don't understand how. The welding requirement looks like it comes from storage_battery via a sequence of copy-froms, and I don't see how my change would have affected that...

@LetterShapedGlyphs
Copy link
Contributor Author

storage_battery_removable has a requirements without a using.
With the original code, the lack of a using would trigger the attached else, which would cause the reqs container to be replaced.
Your change uses emplace_back to add a possible in-line requirement, causing the copied welder to persist.

@jbytheway
Copy link
Contributor

Makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling (S2 - Confirmed) Bug that's been confirmed to exist Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants