Skip to content

Clothing modifications #11160

Merged
merged 36 commits into from Feb 13, 2015

10 participants

@PropaneSoup

This is my attempt at improving clothing reinforcing. I've added a new item: the tailor's kit, and given it a new menu. From that menu, you can add pockets and straps to increase the item's storage, line it with fur to increase it's warmth, or pad it with leather or kevlar to increase it's defense. Also, I've implemented a hard cap of two 'modifications' per piece of clothing. I've put this as a [CR] because I'd like some feedback on balancing. I've done a bit myself, so it's at least somewhat close.

A limitation (feature?) that is currently in the system, I have no way of removing modifications once they're on clothing.

TODO:

  • Create Tailor's kit, move over menu and add spawns
  • Reintroduce sewing kit's original reinforce functionality
  • Change naming scheme to be like the gunmods
  • Change functions so that 0 of stat doesn't mean 0 improvement
  • Move implementation to item::tags
  • General code cleaning
PropaneSoup added some commits Feb 1, 2015
@PropaneSoup PropaneSoup Move pre-existing sewing code into a menu. 078a57b
@PropaneSoup PropaneSoup Beginning of new modify sewing menu
(The actual number modifications don't work yet)
f07ad66
@PropaneSoup PropaneSoup Some proper messages and saving
There's still some weird save/load issues, but I have no idea where
they're happening
efcf78f
@PropaneSoup PropaneSoup Add in skill checks and failures
They're all from the original sewing code, the current balancing is
entirely up in the air.
813353f
@PropaneSoup PropaneSoup Add leather padding 2e02aa6
@PropaneSoup PropaneSoup Astyle and fix of parens
Also fixed some things from my copy/paste
c04a95f
@PropaneSoup PropaneSoup Little bit of balancing
It might be close to decent. I also made padded actually do something
a9ce0a0
@PropaneSoup PropaneSoup Now adding Kevlar
Also a few bugfixes
69dceb1
@PropaneSoup PropaneSoup Some fixes b412ef7
@PropaneSoup PropaneSoup Make things work as intended
Now it it all actually works! Now for balancing
20c5662
@PropaneSoup PropaneSoup Proper astyling e54020b
@PropaneSoup PropaneSoup Fix my awful spelling dc34584
@PropaneSoup PropaneSoup Merge branch 'rebaseing-branch' into Clothing-Reinforcing 5db2e19
@Barhandar

and I added a new menu to the sewing function

"Take ten times as much time to repair your clothes after every fight? Y/N"

And as a final note, this is meant to replace the 'survivor ____' crafting recipes, so I'll create another PR removing those should this manage to get in.

There is already a mod to remove the survivor gear. If you're going to remove them from mainline, please kindly do make them a mod instead of judging for entire playerbase by removing them entirely.

and all of the modifiers are using multiplication, so any piece of clothing with say, 0 storage, once I sew pockets on it, will stay zero

clothing_parameter ? multiply : add.

I've removed the ability to generically 'reinforce' clothes

Bad idea unless your menu includes the ability to increase the effective HP of items: which is what reinforcing partially is (the other part is increasing protection, but that's because protection is item-HP-based).

Considering the above, the generic "activate, point at item, repaired/reinforced" option should stay, but don't increase the protection above normal like it does now, because just not having it will be slightly annoying, and having to navigate prompts to just repair clothing will be infuriating.

Maybe add some sort of "tailor's kit" for all the fancy modifications instead? Also, since coverage is very much king: there should be a modification to increase it somewhat.

P.S. I suspect C++ is capable of using bitflags, so maybe move all the practically boolean variables (pocketed, furred, etc) into bitflags instead?

@PropaneSoup

"Take ten times as much time to improve your clothes after every fight? Y/N"

You do need to press enter one more time than normal, I wasn't sure how to do it better than that.

There is already a mod to remove the survivor gear. If you're going to remove them from mainline, please kindly do make them a mod instead of judging for entire playerbase by removing them entirely.

That was merely an impression that I got from a few threads on the forums, obviously any pull request like that is up for discussion

increase the effective HP of items

The biggest problem I had with that were all the prefixes. There's no way 'reinforced fur lined leather padded pair of socks' is optimal. I suppose I could have it like the gunmods and place a little + before or after it to symbolize that it's been altered, and then say what in the item's description.

@Rivet-the-Zombie
CleverRaven member

And as a final note, this is meant to replace the 'survivor ____' crafting recipes, so I'll create another PR removing those should this manage to get in.

This is not happening. Survivor gear has a purpose and it's not going anywhere, even if we can add extra pockets to our jackets.

Considering the above, the generic "activate, point at item, repaired/reinforced" option should stay, but don't increase the protection above normal like it does now, because just not having it will be slightly annoying, and having to navigate prompts to just repair clothing will be infuriating.

This would be required, unless the leather/kevlar padding already does this as well. No losing basic functionality in the process of upgrading.

I suppose I could have it like the gunmods and place a little + before or after it to symbolize that it's been altered, and then say what in the item's description.

This would work fine.

Also this should be applicable to all forms of armor, even stuff that requires a soldering iron to work with.

@Barhandar

You do need to press enter one more time than normal, I wasn't sure how to do it better than that.

As I've said: separate "tailor's kit" item and consequently use_action that has all these, and have normal sewing kit only useable for repairing and possibly regular reinforcement.

The biggest problem I had with that were all the prefixes. There's no way 'reinforced fur lined leather padded pair of socks' is optimal. I suppose I could have it like the gunmods and place a little + before or after it to symbolize that it's been altered, and then say what in the item's description.

DF's way based on config option, maybe? Those who really like long item names can choose prefixes/suffixes, those who don't have it all collapsed into xItemx, *Item*, !!Item!! and the like.

That was merely an impression that I got from a few threads on the forums, obviously any pull request like that is up for discussion

Forums aren't a good representative of equipment, especially with mod that can disable them and otherwise lack of items that can provide comparable protection (especially coverage and environmental protection).

P.S. Oh, speaking of 'furred' modification! Some items (like leather jacket) could really use the ability to have padding added and removed at will, as well as collars. Maybe have that one be moved into separate craftable item(s), "padding", which can be applied? Actually could work for others, too, adding a material (or if it already exists, material_thickness) onto the item for increased protection, for example.

@Barhandar Barhandar and 2 others commented on an outdated diff Feb 6, 2015
// it_armor::storage is unsigned char
- return static_cast<int>( static_cast<unsigned int>( t->storage ) );
+ return (static_cast<int> (static_cast<unsigned int>( t->storage * pockets) ) );
@Barhandar
Barhandar added a note Feb 6, 2015

(t-> storage > 0 ? t-> storage * pockets : t-> storage + pockets)
Functionally equivalent, unless I've forgot the right order again, to if (t-> storage > 0) { t-> storage * pockets; } else { t->storage + pockets }. Would solve the issue of zero-storage items.

And applying one to cloak would make you look like a kender.

@BevapDin
BevapDin added a note Feb 6, 2015

I suggest you add your modification outside of the casts.

The casts are there because casting an unsigned char to signed int involves two (internal) casts: first to signed char, than to signed int. The first cast to signed char can change the number to become negative: unsigned char x = 255; signed char y = x; - y will be -1. Casting to unsigned int first, avoids that problem.

Why did you remove the const from t, you don't change it, so why does it have to be non-const?

if( pockets ) {
...

Also, you don't need that many brackets: x = 1.5; is totally fine, the added brackets in the return statement is similar useless.

@PropaneSoup
PropaneSoup added a note Feb 6, 2015

Why did you remove the const from t, you don't change it, so why does it have to be non-const?

I removed it when my initial implementation was to change t. I'll add it back. And I'll clean up the brackets

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Barhandar Barhandar and 1 other commented on an outdated diff Feb 6, 2015
+ return 0;
+ };
+ itype_id repair_item = "none";
+ std::vector<std::string> plurals;
+ std::vector<itype_id> repair_items;
+ std::string plural = "";
+ if( mod == it
+ || std::find(repair_items.begin(), repair_items.end(), mod->typeId()) != repair_items.end()) {
+ p->add_msg_if_player(m_info, _("This can be used to repair other items, not itself."));
+ return 0;
+ };
+ if(((mod->furred == 1) && (mod->pocketed == 1)) || ((mod->furred == 1) && (mod->leather_padded == 1)) ||
+ ((mod->pocketed == 1) && (mod->leather_padded == 1)) || ((mod->furred == 1)
+ && (mod->kevlar_padded == 1)) ||
+ ((mod->kevlar_padded == 1) && (mod->pocketed == 1)) || ((mod->leather_padded == 1)
+ && (mod->kevlar_padded == 1))) { //This is a mess. Have to block every combo of 2. Probably a better way.
@Barhandar
Barhandar added a note Feb 6, 2015

Bitflag, if possible, and then if it's not a power of two, it's got a second modification.

@BevapDin
BevapDin added a note Feb 6, 2015

Get rid of the brackets around the comparison and of the comparison itself:

if( ( mod->furred && mod->pocketed ) || (mod->furred && mod->leather_padded) || ...

And maybe use temporary variables:

const bool condition0 = mod->furred && mod->pocketed; // aptly named of course
const bool condition1 = mod->furred && mod->leather_padded;
if( condition0 || condition1 || ...

A bit field is also a nice idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@vache
vache commented Feb 6, 2015

With the ability to add armor and other features to existing clothing, I would be in favor of bumping up the difficulty of some of the survivor gear one or two levels, especially assuming that this change would provide tailoring training when used. Top tier fabricated armor, uses fab 9, and is nowhere near as good as the survivor gear available at tailoring 5-8.

A better candidate for removal would be the MBR vest recipes to add/remove plating.

@Barhandar

With the ability to add armor and other features to existing clothing, I would be in favor of bumping up the difficulty of some of the survivor gear one or two levels, especially assuming that this change would provide tailoring training when used. Top tier fabricated armor, uses fab 9, and is nowhere near as good as the survivor gear available at tailoring 5-8.

Disagree. If something is worse than survivor armor, but takes higher skill, it should be taken down a peg. Like scrap suit, which I suspect has the description for flavor alone and doesn't actually change noise you make.

A better candidate for removal would be the MBR vest recipes to add/remove plating.

It's literally just sliding some material into specialized pockets. The only way I can see that being removed is moving that into MBR vest's use_action.

This would be required, unless the leather/kevlar padding already does this as well.

As far as I can see, it's just increasing cut and bash protection, not "HP".

@PropaneSoup

I do like the separate tailor's kit item idea for specialized tasks like these. So my current TODO list:

-Create tailor's kit item, move over the menu, and give it spawns.
-Figure out how the gunmods work and emulate their naming scheme.
-Put back the reinforcing functionality back into the sewing kit.
-See if I can learn how bitflags work.

Did I forget anything? Also, I know very little c++ syntax, the majority of this was copy/paste, so sorry, but cleaning up the code some with bitflags might be beyond my abilities. I'll see when I get there though.

@PropaneSoup PropaneSoup changed the title from [CR] Clothing modifications to [WIP] [CR] Clothing modifications Feb 6, 2015
@vache
vache commented Feb 6, 2015

An issue right now is the top tier tailoring recipes are in skills 5-8, and there's not a whole lot for level 9 (if anything), and nothing for level 10 (which is supposed to be the ideal skill cap).

The MBR vest recipes are pointless as recipes, and when you get right down to it, the core of it is adding a ceramic plate (or kevlar or whatever) modification to a base MBR vest item. If there is a system to add padding or whatever without requiring a sewing kit then that would be ideal.

I disagree with requiring a separate item to do this just to keep the current usage of the old item streamlined.

@Barhandar

So my current TODO list:

You can do task lists using Markdown, like this:

- [x] checked
- [ ] unchecked

You can modify them on the fly to boot, apparently!

I disagree with requiring a separate item to do this just to keep the current usage of the old item streamlined.

And I heavily disagree with adding prompts to heavily-used items where one could make a new item instead.

See if I can learn how bitflags work.

The best way to learn bitflags, as quick googling showed, is to see them used in practice and then apply copypasta.

@vache
vache commented Feb 6, 2015

I would make it to where you can only modify an item that has been fully repaired, so when activating it on a damaged item, it just repairs it. If activating it on a fully repaired item, then it prompts.

@BevapDin BevapDin and 1 other commented on an outdated diff Feb 6, 2015
@@ -28,7 +28,10 @@
static const std::string GUN_MODE_VAR_NAME( "item::mode" );
static const std::string CHARGER_GUN_FLAG_NAME( "CHARGE" );
static const std::string CHARGER_GUN_AMMO_ID( "charge_shot" );
-
+bool furred = 0;
+bool pocketed = 0;
+bool leather_padded = 0; //Move these somewhere reasonable
+bool kevlar_padded = 0;
@BevapDin
BevapDin added a note Feb 6, 2015

These are global variables, they are not item specific, they are shared between all item instances. You have to add them to the header file as class members. However I suggest you use the item::item_tags set instead, or if you need more than a simple boolean use the item::item_vars and access it through item::get_var etc. (see documentation there).

if(item_tags.count("pockets") > 0) {
    // => has pockets
} else {
    item_tags.insert("pockets"); // adds pockets
}

Using item::item_tags has several advantages:

  • No space is wasted. Most item instance won't ever need that member, so why add it to all of them?
  • It prevents stacking of items that only differ in those modifications - currently modified items would stack with unmodified items, items with different item_tags content do not stack.
  • It is automatically saved and loaded, no need for you to do it.
  • No need to initialize them, the default is not in the set, which means false.

Style issue (=personal opinion): booleans should be initialized with another boolean, not an int: bool x = false;

Edit: never mind, I see you added the member declarations, but the declarations here inside the cpp file are still pointless.

@vache
vache added a note Feb 6, 2015

item::has_flag("name") wraps looking for a tag in a set in a bool to make things nice and easy too:
if(has_flag("pockets"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BevapDin BevapDin commented on an outdated diff Feb 6, 2015
@@ -1666,6 +1675,24 @@ std::string item::tname( unsigned int quantity, bool with_prefix ) const
}
}
+ std::string mod_p_text = "";
+ if (pocketed == 1){
+ mod_p_text = rm_prefix(_("<mod_p_adj>pocketed "));
+ }
+
+ std::string mod_f_text = "";
+ if (furred == 1){
+ mod_f_text = rm_prefix(_("<mod_f_adj>fur lined "));
+ }
+ std::string mod_l_text = "";
+ if (leather_padded == 1){
+ mod_l_text = rm_prefix(_("<mod_l_adj>padded "));
+ }
+ std::string mod_k_text = "";
+ if (kevlar_padded == 1){
@BevapDin
BevapDin added a note Feb 6, 2015

It's a boolean, don't compare it to an integer, don't compare it at all, just

if( kevlar_padded ) {
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BevapDin BevapDin commented on an outdated diff Feb 6, 2015
@@ -919,6 +919,10 @@ class item : public JsonSerializer, public JsonDeserializer
long charges;
bool active; // If true, it has active effects to be processed
signed char damage; // How much damage it's sustained; generally, max is 5
+ bool furred; // If it has been warmed. 0 or 1
@BevapDin
BevapDin added a note Feb 6, 2015

0 or 1

No, it will contain true or false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BevapDin BevapDin commented on the diff Feb 6, 2015
src/iuse.cpp
+ rn += rng(2, 6);
+ }
+ if (p->dex_cur > 16) {
+ rn += rng(0, p->dex_cur - 16);
+ }
+
+ if (rn <= 8) {
+ p->add_msg_if_player(m_bad, _("You damage your %s further trying to sew on pockets!"),
+ mod->tname().c_str());
+ mod->damage++;
+ if (mod->damage >= 5) {
+ p->add_msg_if_player(m_bad, _("You destroy it!"));
+ p->i_rem_keep_contents( pos );
+ }
+ } else
+ if (rn <= 10) {
@BevapDin
BevapDin added a note Feb 6, 2015

You can avoid the additional indentation by using else if( ... ) like the existing code already does.

Also this contains many repetitions that should be moved into a separate function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BevapDin
BevapDin commented Feb 6, 2015

And a better place to initialize my new variables in item.ccp. I thought doing so in the header would work, but it didn't for me.

If you use item::item_tags they will be automatically initialized to "not being there" which means false.

I tried putting some addition in the functions in item.ccp, but it couldn't compile, maybe I was going at it wrong.

It would probably easier to read and write like

int warmth() {
    int result = static_cast<...>(t->warmth);
    // add, remove, multiply result
    return result;
}
@PropaneSoup PropaneSoup Add tailor's kit, iuse and spawns. Also moved over the menu
There's something wrong in the json files, I'll look for it later.
ec474f3
@KA101 KA101 commented on an outdated diff Feb 6, 2015
data/json/item_actions.json
@@ -5,6 +5,10 @@
"name" : "Sew"
},{
"type" : "item_action",
+ "id" : "SEW_ADVANCED",
+ "name" : "Sew_ADVANCED"
@KA101
KA101 added a note Feb 6, 2015

name is shown to the player, and translated. Something like "Modify Clothing" would be better.

(The id field is internal; though it's good practice to have id and name match as closely as feasible, SEW_ADVANCED is OK there.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 commented on an outdated diff Feb 6, 2015
data/json/items/tools.json
+ "name": "tailor's kit",
+ "description": "This is a high quality, plastic kit with a variety of needles, some plastic spools for thread, and a few other useful textile tools. Use a tailor's kit to customize your clothing and armor. This uses your tailoring skill.",
+ "price": 1000,
+ "material": ["plastic", "steel"],
+ "weight": 100,
+ "volume": 2,
+ "bashing": -1,
+ "cutting": 0,
+ "to_hit": -2,
+ "max_charges": 400,
+ "initial_charges": 50,
+ "charges_per_use": 1,
+ "turns_per_charge": 0,
+ "ammo": "thread",
+ "revert_to": "null",
+ "use_action": "SEW_ADVANCED"
@KA101
KA101 added a note Feb 6, 2015

Looking below it, the tailor kit could include a mini scissors (include the scissor action & CUT 1), plenty of pins, a seam ripper, etc. Currently it's a double-capacity sewing kit with the custom-modding menu, so requiring it should probably give some idea of what it contains above/beyond the standard kit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BevapDin BevapDin and 3 others commented on an outdated diff Feb 6, 2015
data/json/items/tools.json
@@ -1,4 +1,4 @@
-[
+[
@BevapDin
BevapDin added a note Feb 6, 2015

Your editor added a BOM (http://en.wikipedia.org/wiki/Byte_Order_Mark), the json parser doesn't like them. Most editors can be instructed in the the options to not do that.

@PropaneSoup
PropaneSoup added a note Feb 6, 2015

I just opened it with notepad... Do I need to get Notepad++ or something?

@KA101
KA101 added a note Feb 6, 2015

If you didn't tell Notepad to save it as "all files" or otherwise not mess with the format, it probably did mess you up.

Notepad++ is used by just about everyone (including me!) who handles DDA code on windows; if there's some reason you'd rather not use it, let us know and we'll see about alternates?

@PropaneSoup
PropaneSoup added a note Feb 7, 2015

I got Notepad++ and I solved the BOM issue, but I made a new one.
2015-02-06_2230
I'm sure that's just an option in Notepad++ that I need to enable. Mind letting me know which one?

@KA101
KA101 added a note Feb 7, 2015

No idea, haven't had that problem. Probably umlaut support?

@narc0tiq
narc0tiq added a note Feb 7, 2015

At a guess:
that one

I don't actually use N++ for Cata myself, though, so this was just me opening the file for a test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
PropaneSoup added some commits Feb 6, 2015
@PropaneSoup PropaneSoup Changed the functions
Didn't get to get much done tonight, hope to get a lot more done
tomorrow. Also the functions should be good with values of 0
2b63ad6
@PropaneSoup PropaneSoup Revert this 304ec20
@PropaneSoup PropaneSoup Revert "Revert this"
This reverts commit 304ec20.
2b318b5
@PropaneSoup PropaneSoup Fixed the jsons
I think I'm missing adding sew_advanced somewhere. Cataclysm is still
yelling at me when I load a world
0ec2807
@PropaneSoup PropaneSoup Actually fix json and move Pocketed to item_tags
I believe the new item_tags usuage should work, but I still haven't
found where I'm missing adding sew_advanced
3ab0f0f
@PropaneSoup PropaneSoup Move everything else to item::tags
Using tags was MUCH easier than my initial implementation, many thanks
to BevapDin for that.
ec81681
@PropaneSoup PropaneSoup Add in basic naming 0bd5506
@PropaneSoup

Ok, this is now in a totally working and playable state, so I'll update the OP to reflect the changes. Many thanks to you all for helping me with the code. There's still some cleanup to be done, and I need to find out how to add strings to an item's descriptions, but everything is working as intended.

@PropaneSoup PropaneSoup changed the title from [WIP] [CR] Clothing modifications to [CR] Clothing modifications Feb 7, 2015
@OzoneH3
OzoneH3 commented Feb 8, 2015

There seems to be a problem with the wielded slot, you can modify every item thats in it.

Edit: It would also be nice to see how much pockets/warmth an item gets when modified.

@BevapDin BevapDin and 1 other commented on an outdated diff Feb 8, 2015
@@ -1744,7 +1766,7 @@ std::string item::tname( unsigned int quantity, bool with_prefix ) const
}
if (has_flag("FIT")) {
- ret << _(" (fits)");
+ ret << _(" (fits) ");
@BevapDin
BevapDin added a note Feb 8, 2015

Why this change? Why the change above at line 1634? And below the "(P) " etc. should probably have the space at the front. The other tags have it that way: " (UPS)" and tagtext is added after the item name.

@PropaneSoup
PropaneSoup added a note Feb 8, 2015

That was for readability, but yea, it never occurred to me to just add a space before the (P) like the rest. Will change. I'm not sure what change at 1634 you're talking about, but it could have been from me messing around trying to find a good place to add this. I've been learning by trial and error.

@BevapDin
BevapDin added a note Feb 8, 2015

I meant the splitting of } else { onto 3 lines, including an empty one (new code line 1654). It looks as if you wanted to add something there.

@PropaneSoup
PropaneSoup added a note Feb 8, 2015

Oh yea, that's where my prefixes used to be. Looks like I didn't put everything back like I found it like I thought I did. Fixin'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@PropaneSoup

Unless there are any objections, this PR is ready to go.

@PropaneSoup PropaneSoup changed the title from [CR] Clothing modifications to Clothing modifications Feb 10, 2015
@BevapDin BevapDin commented on an outdated diff Feb 10, 2015
+ plurals.push_back(rm_prefix(_("<plural>rags")));
+ }
+ if (mod->made_of("leather")) {
+ repair_items.push_back("leather");
+ plurals.push_back(rm_prefix(_("<plural>leather")));
+ }
+ if (mod->made_of("fur")) {
+ repair_items.push_back("fur");
+ plurals.push_back(rm_prefix(_("<plural>fur")));
+ }
+ if (mod->made_of("nomex")) {
+ repair_items.push_back("nomex");
+ plurals.push_back(rm_prefix(_("<plural>nomex")));
+ }
+ if (mod->made_of("plastic")) {
+ repair_items.push_back("plastic");
@BevapDin
BevapDin added a note Feb 10, 2015

I think that should be "plastic_chunk", repair_items contains item ids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BevapDin BevapDin and 1 other commented on an outdated diff Feb 10, 2015
+ plurals.push_back(rm_prefix(_("<plural>fur")));
+ }
+ if (mod->made_of("nomex")) {
+ repair_items.push_back("nomex");
+ plurals.push_back(rm_prefix(_("<plural>nomex")));
+ }
+ if (mod->made_of("plastic")) {
+ repair_items.push_back("plastic");
+ plurals.push_back(rm_prefix(_("<plural>plastic")));
+ }
+ if (mod->made_of("wool")) {
+ repair_items.push_back("felt_patch");
+ plurals.push_back(rm_prefix(_("<plural>wool")));
+ }
+ if (repair_items.empty()) {
+ p->add_msg_if_player(m_info, _("Your %s is not made of fabric, leather, fur, or wool."),
@BevapDin
BevapDin added a note Feb 10, 2015

Maybe add "or plastic" here as it can be used on plastic items as well. Not that it bothers me, but some player will nitpick this (-:

@PropaneSoup
PropaneSoup added a note Feb 10, 2015

Thanks for looking all this over and helping me out BevapDin! I'll fix these as soon as I get back to my computer. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 self-assigned this Feb 10, 2015
@KA101 KA101 commented on an outdated diff Feb 10, 2015
const auto t = find_armor_data();
- if( t == nullptr ) {
+ if( t == nullptr )
@KA101
KA101 added a note Feb 10, 2015

DDA code needs braced, thanks. We aim to be earthquake-proof. (And, more seriously, bracing helps ensure specificity and readability.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 and 2 others commented on an outdated diff Feb 10, 2015
// it_armor::storage is unsigned char
- return static_cast<int>( static_cast<unsigned int>( t->storage ) );
+ int result = (static_cast<int> (static_cast<unsigned int>( t->storage ) ) );
+
+ if (item::item_tags.count("pocketed") > 0){
+ pockets = 1.5;
+ if (result > 0)
+ return result * pockets;
+ else return result + pockets + 3.5; // Can I just use return 5?
@KA101
KA101 added a note Feb 10, 2015

OK, I added the Pockets to my pair of jeans. Result: +1 storage, so I'm worse off enhancing the jeans than I'd be enhancing leather pants or a nanoskirt.

Suggestion: perhaps this should increase the storage (and the encumbrance!) by the target clothing's coverage percentage, minimum increase on each = 1. (So you can add even more pockets to your cargo pants, but they'll get in the way; and no fair giving a miniskirt storage 5.) Can't be used on FANCY or SUPER_FANCY gear, as the pockets would just undermine the look.

(Side note: watch that lack of bracing in the if (result > 0) line.)

@narc0tiq
narc0tiq added a note Feb 10, 2015

While we're discussing nitpicks, what's with the indentation in this snippet?

@Barhandar
Barhandar added a note Feb 10, 2015

Can't be used on FANCY or SUPER_FANCY gear, as the pockets would just undermine the look.

Unless your tailoring is 10+. Then you can pull it off. And just tear off the FANCY and downgrade SUPER_FANCY before that, with a warning prompt.

@KA101
KA101 added a note Feb 10, 2015

If tearing off tags works, sure--I never had any luck with it--but 10 is the cap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 and 1 other commented on an outdated diff Feb 10, 2015
- }
+ if( mod == it
+ || std::find(repair_items.begin(), repair_items.end(), mod->typeId()) != repair_items.end()) {
+ p->add_msg_if_player(m_info, _("This can be used to repair other items, not itself."));
+ return 0;
+ };
+ if(((mod->item_tags.count("furred")) && (mod->item_tags.count("pocketed"))) ||
+ ((mod->item_tags.count("furred")) && (mod->item_tags.count("leather_padded"))) ||
+ ((mod->item_tags.count("pocketed")) && (mod->item_tags.count("leather_padded"))) ||
+ ((mod->item_tags.count("furred")) && (mod->item_tags.count("kevlar_padded"))) ||
+ ((mod->item_tags.count("kevlar_padded")) && (mod->item_tags.count("pocketed"))) ||
+ ((mod->item_tags.count("leather_padded")) && (mod->item_tags.count("kevlar_padded")))) { //This is a mess. Have to block every combo of 2. Probably a better way.
+ p->add_msg_if_player(m_info,_("You can't modify this more than twice."));
+ return 0;
+ };
+ int choice = menu(true, _("How do you want to modify it?"), _("Add extra straps and pockets"),
@KA101
KA101 added a note Feb 10, 2015

Folks who damage their gear trying to modify it might not be thrilled to realize that the tailor kit doesn't permit repairs. Might want to add a repair/reinforce option if the target clothing isn't reinforced.

@PropaneSoup
PropaneSoup added a note Feb 10, 2015

This I disagree with. I considered adding that, but I imagined the tailor's kit to be fairly specialized, not the toolbox of sewing. While I was playtesting, I always happened to carry around a wooden needle anyway, so it was never a huge deal. You make the final call though, so I'll add it if it's a deal breaker.

@KA101
KA101 added a note Feb 10, 2015

If the tailoring kit was somehow separate from the sewing kit, that might be a possibility--I'm not sure what sort of sewing gear would permit adding linings and pockets yet not permit repairing a rip, but whatever.

But the tailoring kit contains a sewing kit, by its recipe, so I'm hard-pressed to understand exactly how one loses the repair functionality. Further, the tailoring kit doesn't specify that it is somehow not suitable for repairs. Folks may have used their sewing kit to make the tailor kit, and would not be pleased to learn that now they're expected to survivalcraft or scrounge another kit to regain their ability to repair.

Rivet's earlier point remains sound:

No losing basic functionality in the process of upgrading.

@PropaneSoup
PropaneSoup added a note Feb 10, 2015

Fair enough, I should have that ready to go later tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101
KA101 commented Feb 10, 2015

OK, the general idea has potential but between the syntax issues, the balance problem, and the lack of repair-ability in the tailor kit, this isn't yet ready for prime time. Sorry, PropaneSoup, but keep at it, please.

@KA101 KA101 removed their assignment Feb 10, 2015
@PropaneSoup

The new way pocketed storage is calculated is the items (volume * coverage) / 100. That then gets added to whatever storage it already had. So jeans go from 2 to 9 storage and a backpack goes from 40 to 42. I thinks that's a good, reasonable increase, but not enough to warrant an increase in encumbrance.

@BevapDin BevapDin commented on an outdated diff Feb 10, 2015
data/json/recipes/recipe_others.json
@@ -326,6 +326,39 @@
]
},{
"type" : "recipe",
+ "result": "tailors_kit",
+ "category": "CC_OTHER",
+ "subcategory": "CSC_OTHER_TOOLS",
+ "skill_used": "tailor",
+ "difficulty": 3,
+ "time": 15000,
+ "reversible": true,
+ "autolearn": false,
+ "book_learn": [[ "textbook_tailor", 3 ] , [ "manual_tailor", 3 ]],
+ "tools": [
+ [
+ ["soldering_iron", -5]
@BevapDin
BevapDin added a note Feb 10, 2015

This should probably be a positive number to make it consume charges. Negative numbers indicates item counts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@PropaneSoup

Unfortunately, my coding inexperience has really been a hindrance. The only way I could get this to work was to wrap the whole thing in another switch, and then copy/paste in the code from the regular sewing. I wanted to just call that function, but I couldn't figure out how. If that needs to be changed, you're going to need to explain how to do it slowly, with small words.
Oh, and I astyled the whole thing again to fix all the weird indents. That's why there are a lot of spaces and tab changes in the commit.

@BevapDin

I wanted to just call that function, but I couldn't figure out how.

Maybe make it like iuse::shishkebab_off or other functions that use iuse_knife. Just pass the same values that sew_advanced got, see how the other knife-like functions do it, for example iuse::zweifire_off. You have to change the function declaration to include the parameters names: int iuse::sew_advanced(player *p, item *it, bool t, point pos) so you can forward them.

@KA101 KA101 self-assigned this Feb 11, 2015
@KA101 KA101 commented on the diff Feb 11, 2015
data/json/recipes/recipe_others.json
@@ -326,6 +326,39 @@
]
},{
"type" : "recipe",
+ "result": "tailors_kit",
+ "category": "CC_OTHER",
+ "subcategory": "CSC_OTHER_TOOLS",
+ "skill_used": "tailor",
+ "difficulty": 3,
+ "time": 15000,
+ "reversible": true,
+ "autolearn": false,
+ "book_learn": [[ "textbook_tailor", 3 ] , [ "manual_tailor", 3 ]],
+ "tools": [
+ [
+ ["soldering_iron", 5]
@KA101
KA101 added a note Feb 11, 2015

This handles the plastic_chunk comment. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 and 1 other commented on an outdated diff Feb 11, 2015
+ if (item::item_tags.count("pocketed") > 0){
+ pockets = (volume() * get_coverage()) / 100;
+ return result + pockets;
@KA101
KA101 added a note Feb 11, 2015

Problem: this also applies to cargo pants. Pocketed Kevlar'd army pants compare favorably to survivor cargo pants, except for a lack of environmental protection.

I'm thinking cap: if the clothing already has more than half its volume in storage, pockets is cut by half. There simply isn't room for more storage.

@PropaneSoup
PropaneSoup added a note Feb 11, 2015

Good idea, typing it up now.

@KA101
KA101 added a note Feb 11, 2015

Further thought: more like pockets * 0.75. Survivor gear is generally considered top/line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 removed their assignment Feb 11, 2015
@KA101 KA101 commented on an outdated diff Feb 11, 2015
@@ -1238,6 +1238,26 @@ std::string item::info(bool showtext, std::vector<iteminfo> *dump, bool debug) c
dump->push_back(iteminfo("DESCRIPTION",
_("This piece of clothing allows you to see much further under water.")));
}
+ if (is_armor() && item_tags.count("pocketed")) {
+ dump->push_back(iteminfo("DESCRIPTION", "--"));
+ dump->push_back(iteminfo("DESCRIPTION",
+ _("You've sewed on some pockets and straps to give you some more places to carry things.")));
+ }
+ if (is_armor() && item_tags.count("furred")) {
+ dump->push_back(iteminfo("DESCRIPTION", "--"));
+ dump->push_back(iteminfo("DESCRIPTION",
+ _("You've sewed in a fur lining to increase its overall warmth.")));
@KA101
KA101 added a note Feb 11, 2015

Lumikant on the IRC: sewed -> sewn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on an outdated diff Feb 11, 2015
data/json/mapgen/mall.json
@@ -64,7 +64,8 @@
["briefcase", 35],
["gold_watch", 20],
["pocketwatch", 20],
- ["collarpin", 10]
+ ["collarpin", 10],
+ ["tailors_kit", 40]
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

Fix your indention please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on an outdated diff Feb 11, 2015
@@ -1753,7 +1773,18 @@ std::string item::tname( unsigned int quantity, bool with_prefix ) const
if (is_tool() && has_flag("USE_UPS")){
ret << _(" (UPS)");
}
-
+ if (item_tags.count("pocketed") > 0 ){
+ ret << _(" (P)");
+ }
+ if (item_tags.count("furred") > 0 ){
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

Again, indention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on an outdated diff Feb 11, 2015
+ if (item::item_tags.count("pocketed") > 0){
+ pockets = (volume() * get_coverage()) / 100;
+ return result + pockets;
+ }
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

The else goes on the same line as the preceeding }
The code after the else must have brackets around it {}
The bracket after the else goes on the same line as the else.
You need to fix this in a lot of places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on an outdated diff Feb 11, 2015
return 0;
}
// it_armor::warmth is signed char
- return static_cast<int>( t->warmth );
+ int result = static_cast<int>( t->warmth );
+
+ if (item::item_tags.count("furred") > 0){
+ warmed = 2; // Doubles an item's warmth
+ if (result > 0){
+ return result * warmed;
+ }
+ else return result + (warmed * 5);
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

These elses need fixing as I outlined above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on an outdated diff Feb 11, 2015
@@ -2451,7 +2501,12 @@ int item::bash_resist() const
if (is_null()) {
return resist;
}
-
+ if (item::item_tags.count("leather_padded") > 0){
+ l_padding = 1.4;
+ }
+ if (item::item_tags.count("kevlar_padded") > 0){
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

indention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on an outdated diff Feb 11, 2015
@@ -2747,160 +2747,582 @@ int iuse::sew(player *p, item *it, bool, point)
add_msg(m_info, _("You can't see to sew!"));
return 0;
}
- int thread_used = 1;
+ int thread_used = 1;
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

It looks like you broke the indention of this whole section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kevingranade kevingranade commented on the diff Feb 11, 2015
src/iuse.h
@@ -77,6 +77,7 @@ class iuse
// TOOLS
int sew (player *, item *, bool, point);
+ int sew_advanced (player *, item *, bool, point);
@kevingranade
CleverRaven member
kevingranade added a note Feb 11, 2015

~how advanced is it?!?~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@PropaneSoup

Current balancing:

http://i.imgur.com/ldqmHjD.png

http://i.imgur.com/rah7y24.png

http://i.imgur.com/eOyNsYP.png

As in the first image, the improved cargo pants are similar to the survivor cargo pants, I think this is alright, as it's a good amount of resources and skill to get to that point.
Second image, the survivor trenchcoat is head and shoulders above the improved regular trenchcoat. That might just be a problem with how strong the survivor trenchcoat is, but I like where the improved regular one is at, stat wise.
Third image, backpack v. backpack, the improved one is a little more defensive, but 10 less storage. Also the fact that it doesn't have any warmth when the regular backpack does looks like a problem with the survivor backpack item.
All in all, I think it's much better than where it was.

@Barhandar

Also the fact that it doesn't have any warmth when the regular backpack does looks like a problem with the survivor backpack item.

Backpacks insulate your back, leading to heat. It's rather annoying, so survivor version would ostensibly be crafted to alleviate that.

All in all, I think it's much better than where it was.

Minor nitpick: first-person descriptions don't match the rest, despite fitting the theme. Imagine getting an upgraded item from NPC trading - you won't be the one who made it, so why would it describe "you" as whoever upgraded it? To match the rest, it'd need to be something like "This gear has had additional pockets and straps sewn on for more places to carry things." and "This gear has been strategically reinforced with kevlar to protect more without additional encumbrance.".

@PropaneSoup

Minor nitpick: first-person descriptions don't match the rest

A fair nitpick though, and easy to fix.

@KA101 KA101 self-assigned this Feb 12, 2015
@KA101 KA101 commented on the diff Feb 13, 2015
src/item.cpp
+ if (item::item_tags.count("pocketed") > 0){
+ pockets = ((volume() * get_coverage()) / 100) / 1.333;
@KA101
KA101 added a note Feb 13, 2015

Hmm. Why the need to divide by 1.333?

@PropaneSoup
PropaneSoup added a note Feb 13, 2015

Further thought: more like pockets * 0.75. Survivor gear is generally considered top/line

I wanted to keep the trend of dividing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@KA101 KA101 merged commit 28ac78f into CleverRaven:master Feb 13, 2015

1 check passed

Details default
@MattAmmann

This looks awesome! One thing I noticed, though, is that kevlar and leather add the same amount of bash protection in item::bash_resist(). Was that intended?

@PropaneSoup

Glad you like it! And yes, that was intended. The kevlar vest has 9 bash resist and 18 cut, while the leather apron has bash 9 and cut 9, and while the rest of the leather equipment varies in how strong it is, I figured that kevlar was a rare enough resource that it should be equal to leather in bash.

@PropaneSoup PropaneSoup deleted the PropaneSoup:Clothing-Reinforcing branch Feb 13, 2015
@KA101
KA101 commented Feb 13, 2015

Welcome to the open source community, incidentally. :-)

@PropaneSoup

Thanks! Next goal: Get something merged without 50+ comments on it...

@KA101
KA101 commented Feb 13, 2015

Hey, you got attention, though--could be one of mine from back in the day, where I ask for input and nobody cares enough to comment.

@Rivet-the-Zombie
CleverRaven member

50+ comments means people are interested in your PR.

It's not a bad thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.