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] [DONE] Rechargeable Tools #5054

Merged
merged 17 commits into from Dec 15, 2013

Conversation

Projects
None yet
6 participants
@desrik
Copy link
Contributor

commented Dec 14, 2013

This adds a few things:

New vpart location:
on_cargo - Will place said item on top of 'center' location

New flags:
'INTERNAL' vpart flag - Requires part to be installed on the same tile as a vpart with the 'CARGO' flag.

'RECHARGE' vpart flag - Recharge items placed inside that have the same flag.
'RECHARGE' item flag - Get recharged if on the same tile as a vpart with the same flag.

New item:
recharge station - This is the new vpart that allows the vehicle battery to recharge tools with the 'RECHARGE' flag.
rechargeable battery mod - Turns tool it is applied to into one able to be recharged with the recharge station. The tool WILL still 'U'nload, but the player receives no batteries.

desrik added some commits Dec 14, 2013

Added a rechargeable battery pack mod for tools, rechargeing station …
…for vehicles, added 'INTERNAL' vpart flag which requires the part to be installed on a tile with a 'CARGO' vpart and 'RECHARGE' which has a twin item flag, which allows the rechargeing vpart to charge a tool with the same flag..
@KA101

This comment has been minimized.

Copy link

commented on src/map.cpp in 7f45d1d Dec 14, 2013

The slang is humorous but will likely give the translators (or at least Wuzzy) fits. Consider either changing it, or adding a commented note explaining the joke?

This comment has been minimized.

Copy link

replied Dec 14, 2013

I don't see that this fits anywhere except in a debug message. It is not something I'd ever display to a player.

Edit: @KA101, why you comment on commit instead of commenting on diff attached to pull request? Sure, github's a little clever, but this line of code is no longer in the pull request.

This comment has been minimized.

Copy link
Owner Author

replied Dec 14, 2013

This line should be gone in current PR.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2013

Interesting. So once a tool can be recharged, it effectively can't be unloaded.

I like the general idea; my codebase is borked ATM so can't test (the critter rework) but this could be pretty nifty. Looking forward to it going into action! :-)


modded->item_tags.insert("RECHARGE");
modded->curammo = dynamic_cast<it_ammo*>(itypes["null"]);
g->add_msg_if_player(p,_("You insert the rechargeable battery pack into your your %s!"), tool->name.c_str());

This comment has been minimized.

Copy link
@infectedmochi

infectedmochi Dec 14, 2013

Contributor

There should be only one "your" here.

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2013

Well my original intention had been to create actual rechargeable versions of certain tools, but IRC discussion didn't seem like they thought that was a good idea... Thus, this :) I actually also just realized that I need to put in something so the 'DOUBLE_AMMO' and 'RECHARGE' will play nice together :P


if (modded->has_flag("RECHARGE"))
{
g->add_msg_if_player(p,_("That item has already has a rechargeable battery pack."));

This comment has been minimized.

Copy link
@infectedmochi

infectedmochi Dec 14, 2013

Contributor

Unless I'm mistaken, it should be "That item has already had......"

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2013

Ok should be good, but I haven't actually tested it fully. Getting ready to start that now.

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2013

@KA101 What slang were you referring to?

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2013

OK turns out it is spitting out batteries on 'U'nload. Need to fix that.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2013

The slang right at that line note? "chargin' lazor"?

Should probably have it tell the player that the mod makes it unable to dispense batteries in future. Otherwise people will see it as a way to get batteries/charge out of their vehicle, and be Disappointed. Changing the installation text to read something like "You convert your %s to run on a rechargeable battery pack, rather than conventional batteries" might work but feels clunky.

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2013

Ah yeah :) That was a line @atomicdryad gave me in IRC to test something. Will commit it out in a few.

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2013

Ok that's set. Next step is a recipe for both the recharge station and the rechargeable batter mod. Any ideas for recipes let me know. I'm off for now.

"symbol": ";",
"color": "light_green",
"name": "rechargeable battery mod",
"description": "This is a patchwork capacitance device made with spare electronics. With enough electronics skill, you could attach this to your devices to be able to rechage it with a recharging station. This mod will also make the tool non-unloadable.",

This comment has been minimized.

Copy link
@NaturesWitness

NaturesWitness Dec 14, 2013

Contributor

Grammar on this could still use a little help. Maybe try something like "A homemade, rechargeable power cell built from salvaged electronics. With enough electronics skill, you could attach it to an electric-powered device to provide it with energy. The power cell is not compatible with standard batteries; it must be re-energized via a special recharging station.

"type":"GENERIC",
"id": "recharge_station",
"name": "recharging station",
"description": "A small box with jacks for different rechargeable tools.",

This comment has been minimized.

Copy link
@NaturesWitness

NaturesWitness Dec 14, 2013

Contributor

Instead of this saying it has charging jacks, why not describe it as an induction charger? It sounds cool, and also fits better with how this works (you just leave compatible tools in the same square as the charger and they refill)

@kevingranade

This comment has been minimized.

Copy link
Member

commented Dec 15, 2013

Cool, I really like where this is going.

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2013

Unless anyone else has any more objections or suggestions, this is ready for merge.

if (is_tool() && has_flag("RECHARGE"))
{
dump->push_back(iteminfo("DESCRIPTION", "\n\n"));
dump->push_back(iteminfo("DESCRIPTION", _("This tool has a rechargeable battery pack. It is not unloadable.")));

This comment has been minimized.

Copy link
@NaturesWitness

NaturesWitness Dec 15, 2013

Contributor

"Unloadable" is sort of an awkward sounding word. A better wording might be "This tool has been modified to use a rechargeable power cell and is not compatible with standard batteries."

}
if (!modded->is_tool())
{
g->add_msg_if_player(p,_("You can only mod tools with this battery mod."));

This comment has been minimized.

Copy link
@NaturesWitness

NaturesWitness Dec 15, 2013

Contributor

Having the word "mod" twice in one sentence looks a little strange. Maybe try "This mod can only be used with tools."

@NaturesWitness

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2013

This is an awesome idea, I've been hoping someone would add something like this for AGES. Batteries in Cata are so goofed up, and the inventory system is so complex, that nobody has wanted to try and write any fixes for it. This PR is a huge step in the right direction.
I especially like how it uses item mods and flags instead of making copies of every tool, this will provide a huge amount of forward compatibility for any other electric tools that get added in the future and cuts down on redundant items which is always a good thing (says the person whose Heatblades PR added more redundant items and code than any other addition in the history of Cata, lol). I hope my suggestions on the wording were helpful, grammar is never easy, especially in a language like English.

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2013

@NaturesWitness Believe it or not my first implementation was exactly what you had described... Rechargeable version of every tool that would have been rarer than normal tools due to their OPness :) IRC discussion is really what made me change my mind.

@NaturesWitness

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2013

I'm not surprised your first thought was to make variants of the items, it's what I would have done. I am glad IRC convinced you to go with this method though, and that you put the extra effort in to make it happen. This system is far more robust, I can't wait to see if the battery mod will work on things like the stun gun or tactical tonfa.

Also, with the "RECHARGE" flag for items, we can easily make other items that use this charging system by default, which opens a lot of options for new stuff. For example, we could have a few variants of items that have this flag by default and so can only be refilled at the charging station, but since they're "one-off" items they can be rarer and have other bonuses to compensate, like storing more power than their equivalent items.
A few good candidates for this could be a portable spotlight (brighter than flashlight, holds more power, but slightly heavier), or a rechargeable-only UPS (stores more power and is more compact than standard UPS, but can't be reloaded instantly in the field like a normal UPS).

@desrik

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2013

This should be usable on anything that the double battery mod will work on. I would love to see this system be used for more than just tools. The 'RECHARGE' flag can be used for other things yes, but so can the 'INTERNAL' flag.

src/map.cpp Outdated
if (it->has_flag("RECHARGE") && next_vehicle->part_with_feature(*part_index, VPFLAG_RECHARGE) &&
next_vehicle->recharger_on) {
if (it->is_tool() && static_cast<it_tool*>(it->type)->max_charges > it->charges ) {
if (it->recharging < 10) {

This comment has been minimized.

Copy link
@kevingranade

kevingranade Dec 15, 2013

Member

Instead of rigidly recharging once every 10 turns, just recharge with a chance of one_in(10). In the long run it works out the same, and we aren't dragging around the "recharging" member for items just for this.

This comment has been minimized.

Copy link
@desrik

desrik Dec 15, 2013

Author Contributor

No problem. I didn't know that's what that function was for. Now to learn how to use it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.