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

Fix for being unable to charge UPS and use UPS-modded tools without UPS #15116

Merged
merged 5 commits into from Feb 1, 2016

Conversation

Core0verload
Copy link

Battery-powered UPS is now rechargeable by default. It can be charged in any recharging station, but you can't unload it anymore. The same applies for two items that are supposed to have rechargeable power cells: e-tonfa (it's stated in description) and control laptop (regular laptop has rechargeable cells in it's disassembly). However, you can't charge a plutonium-powered UPS that way.

All UPS-modded tools now could be used without UPS, making UPS mod a complete replacement for rechargeable battery mod, as it was intended.

UPS charging station is added to Humvee's box. Other mil. vehicles already have it.

Fixes #14976.

@Core0verload
Copy link
Author

Somebody PR something that gives us a rechargeable UPS option - could be a new UPS-specific mod or a new variant of the UPS item itself - and I'll mergetest it.

@Rivet-the-Zombie, here we go.

@Rivet-the-Zombie
Copy link
Member

Thank you, @Core0verload! I'll get on this ASAP.

@Rivet-the-Zombie Rivet-the-Zombie self-assigned this Jan 31, 2016
@Rivet-the-Zombie
Copy link
Member

The game gives no prior warning that you can't remove batteries once you load them into a UPS, so that's going to be a surprise for unwary people. Please include something in the item's description to let them know.

Other than that, everything is fine.

@Rivet-the-Zombie Rivet-the-Zombie removed their assignment Jan 31, 2016
@TheRafters
Copy link
Contributor

:/ I still can't see the benefit of the change. Originally ups could be
loaded and unloaded. Adding the rechargeable mod to a ups made it
un-unloadable (probably un-loadable too, I never tried). Which was fine, it
worked with minimum headaches. Change for the sake of change isn't great
imo, n I would have spoken up against this one if it had been discussed.
Discussion would have at least given me some idea of the perceived benefit
of the change.
On Jan 31, 2016 4:10 PM, "Angela Graves" notifications@github.com wrote:

The game gives no prior warning that you can't remove batteries once you
load them into a UPS, so that's going to be a surprise for unwary people.
Please include something in the item's description to let them know.

Other than that, everything is fine.


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

@Rivet-the-Zombie
Copy link
Member

A UPS-specific recharging mod would be a good fix for this whole situation; the vanilla UPS wouldn't have to change, and it would give us more options for UPS power.

@TheRafters
Copy link
Contributor

This ^ . I've been struggling to articulate exactly what I objected to
about the changes and that covers it nicely. We lost control of the ups
with this change. The changes to the recharging mod are fine, I can live
with them, but there wasn't anything wrong with the ups functionality
before this change.
On Jan 31, 2016 5:27 PM, "Angela Graves" notifications@github.com wrote:

A UPS-specific recharging mod would be a good fix for this whole
situation; the vanilla UPS wouldn't have to change, and it would give us
more options for UPS power.


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

@kevingranade
Copy link
Member

kevingranade commented Feb 1, 2016 via email

@Rivet-the-Zombie
Copy link
Member

I think the gist is that folks want three varieties of UPS:

One that you load/unload with batteries but can't recharge with a charging station.
One that you load/unload with plutonium cells but can't recharge with a charging station.
One that you charge with a charging station but can't load/unload otherwise.

My suggestion:
We already have the first two types. We can add a UPS-specific mod that works on the first type and turns the first type into the third type.

Did that cover the basics, @Malkeus?

@TheRafters
Copy link
Contributor

That is not what I'm asking for at all. The original way it worked was a
ups without a recharge mod allowed batteries to be loaded and unloaded.
With the recharge mod you could neither load or unload, it only gained
charge in the recharge station.
That option to operate on either batteries or car power is what was lost.
If there was an exploit there I didn't know about it. Which circles right
back around to things that would have been mentioned during a discussion
which never took place.
On Jan 31, 2016 6:34 PM, "Kevin Granade" notifications@github.com wrote:

I might not be following, but is the functionality you want back that the
UPS can create batteries on demand as long as it's charged? If so that is
precisely the thing we wanted to get rid of. The UPS isn't a box you put
batteries in, it's a battery pack itself, so you should be charging and
discharging it, not adding and removing batteries.


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

@Core0verload
Copy link
Author

It's not like unloading UPS is used very often. And everybody was using them with rechargeable mod anyway, so it makes sense to make it rechargeable by default. I don't understand that "we lost control" thing at all. You still have your UPS, you can charge it with batteries without ever touching a recharging station if you want.

Please include something in the item's description to let them know.

Done.

@TheRafters
Copy link
Contributor

@rivet yep, that covers it. I often find newer characters have more
electronics than batteries to run then with. A ups that's a battery black
hole would require a lot of inventory management to avoid loading it with
more than you can afford to spare. A reload dialogue that let you specify
how many batteries to load would mitigate this somewhat, instead of forcing
you to drop all of them but the ones you wanted to load. Not ideal though.
On Jan 31, 2016 6:46 PM, "MalkeX" mst3ktoo@gmail.com wrote:

That is not what I'm asking for at all. The original way it worked was a
ups without a recharge mod allowed batteries to be loaded and unloaded.
With the recharge mod you could neither load or unload, it only gained
charge in the recharge station.
That option to operate on either batteries or car power is what was lost.
If there was an exploit there I didn't know about it. Which circles right
back around to things that would have been mentioned during a discussion
which never took place.
On Jan 31, 2016 6:34 PM, "Kevin Granade" notifications@github.com wrote:

I might not be following, but is the functionality you want back that the
UPS can create batteries on demand as long as it's charged? If so that is
precisely the thing we wanted to get rid of. The UPS isn't a box you put
batteries in, it's a battery pack itself, so you should be charging and
discharging it, not adding and removing batteries.


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

@TheRafters
Copy link
Contributor

Who is this everyone you speak of? I agree that unloading is not an issue
in mid to late game, but it comes up a lot in the first few days of my
playthroughs.
On Jan 31, 2016 6:51 PM, "Core0verload" notifications@github.com wrote:

It's not like unloading UPS is used very often. And everybody was using
them with rechargeable mod anyway, so it makes sense to make it
rechargeable by default. I don't understand that "we lost control" thing at
all.


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

@Core0verload
Copy link
Author

UPS spawns empty, you can't just loot it for batteries in early game. And using UPS as a power source in the first days isn't very common because you had to find an UPS, some UPS-powered tools and some batteries, with only batteries being common enough for an early survivor to get.

@TheRafters
Copy link
Contributor

Rngesus smiles on us from time to time. Not common doesn't feel like a
valid reason to render something impossible. Kevin cites the fact that it's
a battery pack and thus should be incapable of accepting batteries. I
disagree, there should be a middle ground which works with batteries. I'm
on my phone so I can't look up the description, is Kevin's assertion
accurate as far as the description goes? I seem to recall something about
batteries.
On Jan 31, 2016 7:07 PM, "Core0verload" notifications@github.com wrote:

UPS spawns empty, you can't just loot it for power cells in early game.
And using UPS as a power source in the first days isn't very common because
you had to find an UPS, some UPS-powered tools and some batteries, with
only batteries being common enough for an early survivor to get.


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

@Core0verload
Copy link
Author

This is a unified power supply, or UPS. It is a device developed jointly by military and scientific interests for use in combat and the field. The UPS is designed to power armor and some guns, but drains batteries quickly.

This is UPS's desc.

@TheRafters
Copy link
Contributor

That description is vague enough to support any interpretation. Kevin said
rechargeable battery packs; I've always viewed them as reloadable power box
attached to a wireless power transmitter. The wireless power transmission
device was the specially developed part. I dunno. I digress.

All I'm really asking for here is some supporting logic for the change. And
your unsourced opinion that everyone is using it a certain way is not that.
On Jan 31, 2016 7:55 PM, "Core0verload" notifications@github.com wrote:

This is a unified power supply, or UPS. It is a device developed jointly
by military and scientific interests for use in combat and the field. The
UPS is designed to power armor and some guns, but drains batteries quickly.

This is UPS's desc.


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

@Core0verload
Copy link
Author

And I thought the discussion about storage batteries balance was pointless...

The only reason behind these changes: people were complaining a lot about UPS being non-rechargeable. I didn't even knew it was possible to make it rechargeable and that a lot of people actually used that. So I made it rechargeable. And what's the supporting logic behind your nitpicking?

@TheRafters
Copy link
Contributor

That the whole change was unnecessary precisely because it was already
possible to make them rechargeable. Which is something you just pointed out
that you didn't even know about. It worked before, it worked well and after
you changed it, it was broken. Now your 'fix' leaves it less capable than
it was before.

I like the enhancement to the recharging station. I like combining ups and
recharge mods for tools. Those are both good ideas and enhance the game.
The sole thing I disagree with is the alterations to the ups BC it is now
less capable than its previous incarnation.

On a personal level, the fact that you call discussion nitpicking, get
angry over said discussion AND couldn't be bothered to thoroughly research
and then test your own PR bothers me.

You changed something to make it capable of doing something it was already
capable of doing. And not for the better, imo.
On Jan 31, 2016 8:39 PM, "Core0verload" notifications@github.com wrote:

And I thought the discussion about storage batteries balance was
pointless...

The only reason behind these changes: people were complaining a lot about
UPS being non-rechargeable. I didn't even knew it was possible to make it
rechargeable and that a lot of people actually used that. So I made it
rechargeable. And what's the supporting logic behind your nitpicking?


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

@Core0verload
Copy link
Author

So you think that unloading is a major feature of an UPS, and removing it made UPS a lot less capable than it was before, even if it was replaced with an ability to recharge in station. I don't think so. The further discussion will be pointless.

@TheRafters
Copy link
Contributor

I think you 'fixed' something that wasn't broken. You can be as dismissive as you like, but it doesn't change the FACT that you didn't know the feature you added already existed. And for the record, I never said it was a major feature, nor that it was a lot less capable, don't put words into my mouth. You still haven't given a reason for the change, btw.

@TheRafters
Copy link
Contributor

http://smf.cataclysmdda.com/index.php?topic=12053.msg266096#msg266096
This is some more of that 'pointless' discussion.

@Core0verload
Copy link
Author

If you don't like it, feel free to open a pull that removes ["RECHARGE", "NO_UNLOAD"] from a regular UPS and adds a recipe for converting the regular UPS into a rechargeable.

@TheRafters
Copy link
Contributor

So mote it be.

@Coolthulhu Coolthulhu self-assigned this Feb 1, 2016
@@ -4236,7 +4238,7 @@
"color": "light_green",
"name": "advanced UPS",
"name_plural": "advanced UPS's",
"description": "This is an advanced version of the unified power supply, or UPS. This device has been significantly redesigned to provide better efficiency as well as to consume plutonium fuel cells rather than batteries.",
"description": "This is an advanced version of the unified power supply, or UPS. This device has been significantly redesigned to provide better efficiency as well as to consume plutonium fuel cells rather than batteries. Sadly, it's plutonium reactor can't be charged in UPS charging station.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "its", not "it's".

Coolthulhu added a commit that referenced this pull request Feb 1, 2016
Fix for being unable to charge UPS and use UPS-modded tools without UPS
@Coolthulhu Coolthulhu merged commit c2dde28 into CleverRaven:master Feb 1, 2016
@Core0verload Core0verload deleted the UPS_fix branch February 3, 2016 10:56
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

5 participants