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

(WIP) Gunmod support for crossbows! #5101

Closed

Conversation

Projects
None yet
4 participants
@NaturesWitness
Copy link
Contributor

commented Dec 16, 2013

Adds a new mod category, crossbow. I think this is something people have wanted for a while, and as it turns out it wasn't nearly as hard as i thought it would be. Currently, only the laser sight is flagged as being compatible with crossbows, I used it for testing. I could use some guidance as to what gunmods should be flagged as crossbow-compatible, so please tell me what you think I should add!

Tested and works, I'll enable more gunmods for crossbows and see if anything breaks.

Crossbow mod support and laser sight
Adds a new mod catagory, crossbow.  Also allows laser sight to be
mounted on crossbow, used for testing, more mods will be enabled on
crossbows in later commits.
@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2013

Just realized this doesn't differentiate between crossbows and regular bows, anyone know how to implement this?

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

New flag on crossbows?

Or perhaps check the ammo type? Arrows == Bow, Bolts == Crossbow

@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2013

That's what I'm thinking, I'm just not certain how to implement it.

@@ -8301,6 +8300,10 @@ void player::use(int pos)
g->add_msg(_("That %s cannot be attached to a rifle."),
used->tname().c_str());
return;
} else if (guntype->skill_used == Skill::skill("archery") && !mod->used_on_crossbow) {

This comment has been minimized.

Copy link
@desrik

desrik Dec 16, 2013

Contributor

Yeah I see what you have to do now... You need another else if:

} else if (guntype->skill_used == Skill::skill("archery") && mod->used_on_crossbow && !guntype->ammo == "bolt") {
g->add_msg(_("That %s can only be attached to a crossbow."),
used->tname().c_str());
return;
}

This will prevent any mods from ever being installed on anything that uses arrows for ammo.

@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2013

This doesn't seem to work, still lets me put a laser on a compound bow.

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

See my updated line comment? Sorry about that :)

Although a laser on a compund bow should be allowed :) I have seen them IRL.

@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2013

Adding mods for regular bows was going to be my next PR, I just wanted to keep to categories separate. For example, I want underbarrel mods like bayonets and grenade launchers to work on crossbows, but not on regular bows. I guess these both really need to be sorted out at the same time, huh?

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

In that case you most likely would need to set up the mod categories first. New json field perhaps? "location": "underbarrel", "location": "scope", etc. Then in-code assign different weapon types a set of valid locations. When installing the mod check the mod location against the target weapon types valid mod locations.

@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2013

I like the idea of having mods being based on location on the weapon as opposed to the current blanket fields, but I think something like that is beyond my coding ability. I wonder if I should make an issue request asking if someone could implement it? It would be a big improvement, not just for bows but for all guns.

For the moment, maybe I'll just change this to a more general "archery" mod category that covers both bows and crossbows. That could at least allow laser sight, improved iron sights and scopes for these. Either that, or maybe I can just stick a "IS_CROSSBOW" flag on all the crossbows in the .json, and have it look for it?

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

For now you could just check the id and if it's not one you want compatible then message the player and return. Crude but it will work until mod locations get implemented. Might actually take it on myself here in a bit. Need a break on the construction expansion anyways. Spent around an hour and a half last night coding a deconstruct menu only to find out I need to write a few new functions before it would work :P

@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2013

Right now I'm trying to implement a "CROSSBOW" flag on items in the .json that the code can check for, but it doesn't seem to be working. Here's what I got so far;

} else if (guntype->skill_used == Skill::skill("archery") && !mod->used_on_crossbow) && !gun->item_tags.count("CROSSBOW") {
g->add_msg(_("That %s cannot be attached to a crossbow."),
used->tname().c_str());
return;

What am I missing here? This is the error I'm getting;

C:\Documents and Settings\Mason\My Documents\GitHub\Cataclysm-DDA\src\player.cpp||In member function 'void player::use(int)':|
C:\Documents and Settings\Mason\My Documents\GitHub\Cataclysm-DDA\src\player.cpp|8303|error: expected identifier before '!' token|
C:\Documents and Settings\Mason\My Documents\GitHub\Cataclysm-DDA\src\player.cpp|8303|error: expected ';' before '!' token|
||=== Build finished: 2 errors, 0 warnings (0 minutes, 6 seconds) ===|

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

Try: !gun->has_flag("CROSSBOW")

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

Ah :) Your right @Rivet-the-Zombie. Also seems to be a double left parenthesis. g->add_msg(("That

Oh and @NaturesWitness, here is what I came up with for mod locations:

ammo
auxiliary
barrel
bore
grip
mechanism
side
sights
stock
tip

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Dec 16, 2013

Yeah looks to be simple separator errors.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Dec 16, 2013

Rivet's pedantic nitpick of the day:

axillary - Of or relating to the armpit.

auxiliary - Available to provide extra help, power, etc., when it is needed.

Sorry, that was too funny for me to resist.

@desrik

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2013

Lol. That's the Firefox auto-spellcheck for you :P

@desrik desrik referenced this pull request Dec 16, 2013

Merged

[DONE] Weapon mod rework #5103

@NaturesWitness

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2013

Thanks to desrik's awesome work, this PR is no longer necessary, so I'm closing it.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Dec 18, 2013

I don't think there is one right now, but we've been planning on moving
xbows to use the rifle skill instead of archery, which would provide he
differentiation you want.

@NaturesWitness NaturesWitness deleted the NaturesWitness:crossbow-mods branch Dec 20, 2013

@kevingranade kevingranade changed the title (WIP) Gunmod support for crossbows! (WIP) Gunmod support for crossbows! Sep 20, 2014

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.