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
Unify holster code (part 1) #13982
Merged
Merged
Unify holster code (part 1) #13982
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Callback now also accepts an optional argument for the number of moves already expended wielding the weapon. A single message is now shown when the object is wielded with moves expended both before and within the function now considered when selecting the appropriate wording.
* Pass the number of expended moves to item::on_wield() callback * Refactor check as player::can_wield()
Hope it goes well. |
@chaosvolt Thanks, its consumed a lot of time to date but I think we have finally reached the end |
Hopefully, yeah. Splitting things like this up always makes it easier, so should go by quicker. |
* Check an item can be wielded before removing it from container * Deduct moves according to both relevant skill and item volume * Pass correct number of consumed moves to item::on_wield() * Support containers with more than one contained item
Condensed to include all relevant commits including the improvements suggested by BevapDin
* Document holster_actor in JSON_INFO.md * Also missing entry for NO_QUICKDRAW
* More realistic replacement for storing knives in boots * Both ankle holster and sheath are slow to draw at low skill levels
* Holster demonstrates default usage of holster_actor * Demonstrate storing a pair of volume 1 knives in boots
This was referenced Nov 15, 2015
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As suggseted by @kevingranade in #13936 this is the first part of the original PR. It includes all of the code review suggestions suggested by @BevapDin and others.
This first PR includes only the refactoring within
player::wield
,player::wield_contents
anditem::on_wield
, the newholster_actor
and some demonstration items (ankle sheath, ankle holster, boots and holster).It does not include the following changes:
fire
menu including support for shoulder straps (44ddd13 and 4e77631)The plan would then be to PR parts 2a and 2b simultaneously covering the behavioural changes and the UI improvements. In the case of the latter I'll probably add a cancel option which means we can also close #13599 and possibly also a prompt to holster items instead of dropping them when low on inventory space (#13983).
Finally part 3 would be adding and balancing any new holsters and then removing
iuse::boots
,iuse::sheath_sword
,iuse::sheath_knife
,iuse::holster_gun
,iuse::holster_ankle
. @chaosvolt has expressed some interest in adding new scabbards and perhaps others will also have suggestions