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

Throwing knife bandolier #37577

Closed
wants to merge 18 commits into from

Conversation

ashGlaw
Copy link
Contributor

@ashGlaw ashGlaw commented Jan 31, 2020

Summary

SUMMARY: Content "Adds a bandolier for throwing knives"

Purpose of change

Other weapons and ammo can be stored in wearable containers, and I wanted throwing knives to get some love.

Describe the solution

I added the item to bandoliers.json, and I added the "SHEATH_KNIFE" flag to throwing_knife in archery.json. I made the interaction speed of the bandolier pretty low, but not the lowest of the sheaths/holsters. It should be lower than getting the knives out of inventory anyway.

I also fixed a syntax error in med.json that was keeping the game from loading.

Describe alternatives you've considered

I'm thinking about also making a small wrist or ankle sheath for throwing knives that takes 2 blades. It would be swell if, on activation of the bandolier, one of the available option was to fully load the bandolier all at once.

The holster UI for multi holsters is pretty cumbersome. I worry that people won't bother using the bandolier because it takes more effort

Testing

I spawned the bandolier in the game, loaded it up with throwing knives, drew one and threw it. All worked fine.

Additional context

Currently, I'm not sure if throwing knives are the only knives that can be stored in the bandolier. I'm also not sure if that's an issue. Putting a special flag on throwing knives that's only called by the bandolier could be a solution, but that feels inelegant to me. What do people think?

If people dig it I could make a crafting recipe and add it to lists

ColdandDead and others added 3 commits January 30, 2020 11:00
I'm not 100% on how all the moving parts connect with eachother, so I'm adding the bandolier and a tag to throwing-knifes so I can begin testing
@Brian-Otten
Copy link
Contributor

Did you mean to slightly edit melatonin in the same PR?

Throwing knife bandolier looks good to me, doesn't sound like a problem if it's also able to hold other knives that have the exact same volume as the throwing knives.

Being able to automatically fully load it might be nice, but would handle differently from other sheaths/pouches, so this probably shouldn't be a special exception. Could someday be a nice feature of such items.

@TechyBen
Copy link
Contributor

Best to keep other bugs/edits in a separate PR AFAIK.

@RarkGrames
Copy link
Contributor

RarkGrames commented Jan 31, 2020

You need to add a recipe or item group entries (preferably both). Otherwise it becomes dead code not used anywhere.

ashGlaw and others added 2 commits January 31, 2020 08:51
Remove extra period

Co-Authored-By: Rail-Runner <[email protected]>
Added a crafting recipe, based off of other bandolier recipes
No tests yet, still compiling, but I plan to once I can
@ashGlaw
Copy link
Contributor Author

ashGlaw commented Jan 31, 2020

You need to add a recipe or item group entries (preferably both). Otherwise it becomes dead code not used anywhere.

Recipe's easy. Item groups are something I haven't touched before. I'm not sure where the best place to put it would be. Any advice on where the bandolier belongs?

@ashGlaw
Copy link
Contributor Author

ashGlaw commented Jan 31, 2020

Did you mean to slightly edit melatonin in the same PR?

Yes. The duration was written in as "16 h" when it needed to be an integer. I fixed it so I could test. I can take it out later on

Throwing knife bandolier looks good to me, doesn't sound like a problem if it's also able to hold other knives that have the exact same volume as the throwing knives.

Great. Thanks!

Being able to automatically fully load it might be nice, but would handle differently from other sheaths/pouches, so this probably shouldn't be a special exception. Could someday be a nice feature of such items.

Gree. No need to make this bandolier special, but maybe any holster with multi deserves the option to fully load it with one type of item later on. That's a problem for the future, though.

@anothersimulacrum
Copy link
Member

Yes. The duration was written in as "16 h" when it needed to be an integer. I fixed it so I could test. I can take it out later on

You should update the binary you're using for testing, when I changed it to that I added support for time_duration strings.

ashGlaw added 3 commits January 31, 2020 10:19
Tested the crafting recipe. Worked fine. Linted the recipe file too
Linted bandolier.json to pass tests
@RarkGrames
Copy link
Contributor

Recipe's easy. Item groups are something I haven't touched before. I'm not sure where the best place to put it would be. Any advice on where the bandolier belongs?

I couldn't think of any shop or similar place where it would be reasonable to spawn, but survivor zombies could drop it.

If that is fine for you, you can add an entry in the survivorzed_extra item group in \itemgroups\clothing.json, with a spawn weight of 5.

added [ "throwingknifebandolier", 5 ], to clothing.json in itemgroups, as recommended
@Brian-Otten
Copy link
Contributor

Survivor zed would be a pretty good place for this to spawn, with a low probability.

@ashGlaw
Copy link
Contributor Author

ashGlaw commented Jan 31, 2020

I’ve expanded the pr to include a double throwing knife shesth for the ankle, and a single knife sheath for the wrist, recipes for both, and I added them to the list RarkGrames and Brian-Otten suggested.

The items are tested. I’ll leave testing the remaining two recipes and making sure survivor zombies drop them for the next time I’m at the computer

@ifreund ifreund added <Enhancement / Feature> New features, or enhancements on existing [JSON] Changes (can be) made in JSON Items / Item Actions / Item Qualities Items and how they work and interact Ranged Ranged (firearms, bows, crossbows, throwing), balance, tactics 0.E Content Freeze labels Feb 1, 2020
@Shodan14
Copy link

Shodan14 commented Feb 2, 2020

I think we need more multi-purpose bandoliers at this point.

@ashGlaw
Copy link
Contributor Author

ashGlaw commented Feb 4, 2020

How do you mean?

@Shodan14
Copy link

Shodan14 commented Feb 8, 2020

How do you mean?

Like one bandolier that can hold any type of ammo and one that can hold any throwing items from rocks to knives to grenades.

ashGlaw added 3 commits February 7, 2020 19:40
Finally formatted right, I hope
Fixed a change I made to avoid a crash to conform to master
@ashGlaw
Copy link
Contributor Author

ashGlaw commented Feb 8, 2020

Well, I think one issue is that different ammo/thrown weapons have different volumes, so the amount you can fit into the bandolier would be different for each type of ammo, wouldn't it?

Maybe general bandoliers would work well for the simplified guns mod

@ashGlaw ashGlaw changed the title [WIP] Throwing knife bandolier Throwing knife bandolier Feb 8, 2020
@ashGlaw ashGlaw marked this pull request as ready for review February 8, 2020 03:12
@Shodan14
Copy link

Shodan14 commented Feb 9, 2020

Well, I think one issue is that different ammo/thrown weapons have different volumes, so the amount you can fit into the bandolier would be different for each type of ammo, wouldn't it?

Maybe general bandoliers would work well for the simplified guns mod

If you imagine it just having elastic straps, MOLLE-style you could attach a number of different things.

@ZhilkinSerg
Copy link
Contributor

Feel free to reopen if you want working on it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Enhancement / Feature> New features, or enhancements on existing Items / Item Actions / Item Qualities Items and how they work and interact [JSON] Changes (can be) made in JSON Ranged Ranged (firearms, bows, crossbows, throwing), balance, tactics stale Closed for lack of activity, but still valid.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants