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

Assorted armor changes/additions #13561

Merged
merged 17 commits into from Sep 18, 2015

Conversation

@chaosvolt
Copy link
Contributor

chaosvolt commented Sep 15, 2015

  1. Changes normal quivers to use the waist layer instead of strapped. Belt quivers were/are more common historically, will leave large quivers unchanged.
  2. Added sling pack and whistle. Tweet? Fweeeet.
  3. Added new items to a few professions.
  4. Added modular function for noisemaking, to be used by the new whistle, bicycle horns, and airhorns. Only possible thanks to a SHITLOAD of advice and help from Rivet, Coolthulhu, and BevapDin.
  5. Removed storage of helmet netting, at Kevin's suggestion ( http://smf.cataclysmdda.com/index.php?topic=11141.msg248926#msg248926 ).
  6. Remembered to try that relayering of hoodies again, this time I did both, not just the cotton version.
chaosvolt
1. Changes normal quivers to use the waist layer instead of strapped.
Belt quivers were/are more common historically, will leave large quivers
unchanged.
2. Added sling pack and whistle. Tweet? Fweeeet.
3. Added function for whistles. Wasn't sure how to fully tweak the
function to be modular, which would've allowed merging whistle-blowing
with the bicycle horn function.
4. Added new items to a few professions.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 15, 2015

...doh. ;w;

EDIT: Oh. It was JUST a missing separator? I was afraid I'd fucked up such a simple source tweak.

Chaosvolt
1. Commas are good for your health.
@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

Rivet-the-Zombie commented Sep 15, 2015

One missing separator and the software acts like it's the end of the world.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 15, 2015

Yep. Such fail.

Now, to pose the question to you since someone suggested in on the forums...should I add seaparate versions of the basic quivers as back quivers, or just leave normal/birchbark as belt quivers, and large quivers as a back quiver?

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

Rivet-the-Zombie commented Sep 15, 2015

I don't see as much use for being able to wear the small versions on your back, as I do for being able to wear the large version on your hip.

int iuse::tweet(player *p, item *it, bool, const tripoint &pos)
{
sounds::sound(pos, 38, _("FWEEEET!"));
p->add_msg_if_player(_("You blow the whistle."));

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Sep 15, 2015

Contributor

Generic messages are preferable: p->add_msg_if_player(_("You blow the %s."), it->tname().c_str());

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 15, 2015

Hmm. So should I be consistent and make all of them use the waist slot? >.>

EDIT: And...doh.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 15, 2015

That reminds me, from the looks of it, it seems that use_actions with modular parameters are in iuse_actor. Would need to see how best to do that, or save the task for someone else. >.>

Chaosvolt
1. Whistling changed to use ye olde %s instead. I blame the bicycle horn not being generic. >.<
@kevingranade

This comment has been minimized.

Copy link
Member

kevingranade commented Sep 15, 2015

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

Rivet-the-Zombie commented Sep 16, 2015

Now if only we could have a system where skilled archers could keep a few extra arrows in their draw hand, to allow for faster followup shots.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

And if you fail an intellect roll, you decide upon firing that trying to nock and shoot all of them at once is a good idea?

EDIT: What happens if you bump a reload-and-shoot weapon's capacity above 1? This gives me ideas.

@kevingranade

This comment has been minimized.

Copy link
Member

kevingranade commented Sep 16, 2015

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Would be amusing, yeah. For now though...fweet. o3o

Still would've liked to make the bicycle horn function modular, but...hmm.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

Coolthulhu commented Sep 16, 2015

Making a generic noisemaker function wouldn't be hard.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Hmm. I assume I'll need to first move it to where modular use_actions are?

Added function not yet implement by the items.

1. Wanna bet this is gonna hurt? At least this is a step in the right
direction, so maybe Coolthulhu will know what I'll surely mess up.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Now, to see what inevitable failure shall ensue. ;w;

chaosvolt added 2 commits Sep 16, 2015
chaosvolt
Take 2
1. Oh, so you mean removing more things might unbreak it? We'll see.
chaosvolt
Take 3
1. Not sure if this'll fix anything...
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Augh. Fuck.

It's late and I should sleep. Will see if I can unfuck this in the morning, or get someone to tell me what I'm doing wrong.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

Coolthulhu commented Sep 16, 2015

You changed function's signature rather than just removing the variable identifiers.

You want it to look like:

bool manualnoise_actor::can_use( const player*, const item *it, bool, const tripoint& ) const
{
    return it->charges > it->type->charges_to_use();
}

Function signature has to be the same in the implementation and definition and the override is mandatory.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Ah, I see. Working on it, then. ^^"

chaosvolt
Take 4
And now to see if these changes alone will fix it.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Whee, new and exciting errors.

src/iuse_actor.h:551:22: error: 'can_use' marked 'override' but does not override any member functions
        virtual bool can_use( const item*, bool, const tripoint& ) const override;
                     ^
src/iuse_actor.h:551:22: error: 'manualnoise_actor::can_use' hides overloaded virtual function [-Werror,-Woverloaded-virtual]
src/iuse.h:255:18: note: hidden overloaded virtual function 'iuse_actor::can_use' declared here: different number of parameters (4 vs 3)
    virtual bool can_use( const player*, const item*, bool, const tripoint& ) const { return true; }

Hence why I assumed yanking out override was a good idea. ._.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

Rivet-the-Zombie commented Sep 16, 2015

virtual bool can_use( const item*, bool, const tripoint& ) const override;
virtual bool can_use( const player*, const item*, bool, const tripoint& ) const { return true; }

One of these things is not like the others,
One of these things just doesn't belong.
Can you tell which thing is not like the others,
By the time I finish my song?

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Oh derp. And now that I'm back, time to unfuck that. ;w;

Chaosvolt
1. This is gonna hurt. ;w;
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 16, 2015

Ahah. Ahahaha. Muahahaha.

It actually works now?

EDIT: Now if I could just get a compiler to work right so I can be absolutely sure...

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 17, 2015

Ack. I blame poorly-thought-out copy pasting of code from fireweapon_off, on my part.

Any other strings there that need that treatment?

"use_action": "HORN_BICYCLE"
"use_action": {
"type": "manualnoise",
"use_message": "You honk the %s.",

This comment has been minimized.

Copy link
@BevapDin

BevapDin Sep 17, 2015

Contributor

The "%s" in this string is never replaced.

The string is used at:

p->add_msg_if_player( _(use_message.c_str()) );

But there is no additional parameter that can be used to replace the "%s", can either replace the "%s" in JSON (where you already know what item is invoked), or add a parameter to add_msg_if_player.

"revert_to": "null",
"use_action": {
"type": "manualnoise",
"use_message": "You blow the %s.",

This comment has been minimized.

Copy link
@BevapDin

BevapDin Sep 17, 2015

Contributor

Same problem with the "%s".

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 17, 2015

...doh. Easier to remove the %s in the JSON. >_>

chaosvolt
1. Implemented suggested change to noise message.
2. Made use messages not rely on the %s placeholder, and added a warning
comment noting it's unsupported presently.
@@ -931,7 +929,7 @@ void Item_factory::load_basic_info(JsonObject &jo, itype *new_item_template)
if (jo.has_member("name_plural")) {
new_item_template->name_plural = jo.get_string("name_plural").c_str();
} else {
// default behaviour: Assume the regular plural form (appending an “s”)
// default behaviour: Assume the regular plural form (appending an “s”)

This comment has been minimized.

Copy link
@BevapDin

BevapDin Sep 17, 2015

Contributor

Your editor destroyed the perfectly valid Unicode characters (left and right double quotation mark). Whatever it is now, it does not appear to be valid UTF-8. Please instruct your editor to open and save the source files as UTF-8 (there should be a setting named encoding or similar).

1. Go home Wordpad, you're drunk.
if( t ) {
return 0;
}
if( it->charges <= 0 ) {

This comment has been minimized.

Copy link
@BevapDin

BevapDin Sep 17, 2015

Contributor

Bicycle horns don't have charges, because their action (iuse::horn_bicycle) does not consume any charges and therefor they do not start with any charges (their JSON entry: initial charges, maximal charges, charges per use are all 0).

As you're going to combine the two iuse functions (one for a tool using charges and one for a tool not using charges), you should make sure the old horn still work as they used to work. Like testing for either charges_to_use returning 0 or, charges being large enough:

if( it->type->charges_to_use() != 0 && it->charges < it->type->charges_to_use() ) {
    return 0;
}

bool manualnoise_actor::can_use( const player*, const item *it, bool, const tripoint& ) const
{
return it->charges > it->type->charges_to_use();

This comment has been minimized.

Copy link
@BevapDin

BevapDin Sep 17, 2015

Contributor

This should probably test for => because having exactly the required charges should work fine. Also make sure it works for items that don't consume any charges.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 17, 2015

Ack. I see, how do I ensure that the can_use function works right around chargeless items, like bicycle horns?

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

Rivet-the-Zombie commented Sep 17, 2015

  1. Go home Wordpad, you're drunk.

Wordpad isn't the best choice here. Try Notepad++; it's pretty much the gold standard among us Windows-using DDA devs.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 17, 2015

Doh. Really should get that sometime, though this is amusingly enough the first time that using Wordpad broke something for me. >.<

Chaosvolt
1. Should now check for items that don't use charges, and allow using when charges equal charges-to-use.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 17, 2015

Aw hell.

Chaosvolt added 2 commits Sep 17, 2015
Chaosvolt
1. Oh this is gonna hurt.
Chaosvolt
1. If this works without any idiocy ensuring, I'm gonna be amazed.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 17, 2015

Hah. Funny how both times I referenced that meme in the commit title, fixing did indeed intensify.

I'm going to bet that this fix likely will leave new and exciting problems for BevapDin to point out. Will see in the morning. @_@

Chaosvolt added 2 commits Sep 17, 2015
Chaosvolt
1. Should be a less messy way to make it work as intended, if I implemented BevapDin's suggestion right.
Chaosvolt
1. Helmet netting volume removed. From Kevin's suggestion in a thread concerning power armor ( http://smf.cataclysmdda.com/index.php?topic=11141.msg248926#msg248926 ).
2. Remembered the hoodie relayering I was plotting last time I did armor tweaks. This time remember to do BOTH versions, normal and wool.
@Coolthulhu

This comment has been minimized.

Copy link
Contributor

Coolthulhu commented Sep 18, 2015

EDIT: My fault, I thought it was to the helmet netting

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 18, 2015

¿Que? What'd I miss? o.o

@Coolthulhu Coolthulhu self-assigned this Sep 18, 2015
@Coolthulhu

This comment has been minimized.

Copy link
Contributor

Coolthulhu commented Sep 18, 2015

Oops, looks like it conflicts with the recent change to netting storage.

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 18, 2015

What. Someone already nerfed it for me then? >.>

EDIT: Oh, I see. It was on #13589 it seems.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

Coolthulhu commented Sep 18, 2015

OK, it conflicts with a bit more than that: Kevin's huge json cleanup at #13591

I'll merge Kevin's PR first - it is huge and a lot of json changes would conflict with it. Should be easy to fix this one afterwards (just remove all "storage": 0 and "environmental_protection": 0).

@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 18, 2015

Oh wow. Very well then, sorry about that. ^^"

@Coolthulhu Coolthulhu merged commit 3357af3 into CleverRaven:master Sep 18, 2015
1 check passed
1 check passed
default This has been rescheduled for testing as the 'master' branch has been updated.
@chaosvolt

This comment has been minimized.

Copy link
Contributor Author

chaosvolt commented Sep 18, 2015

Thank you for the merge. ^^

@chaosvolt chaosvolt deleted the chaosvolt:quiver-tweak-and-additions branch Jan 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.