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

Unify holster code (part 1) #13982

Merged
merged 7 commits into from Nov 15, 2015
Merged

Conversation

mugling
Copy link
Contributor

@mugling mugling commented Nov 11, 2015

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 and item::on_wield, the new holster_actor and some demonstration items (ankle sheath, ankle holster, boots and holster).

It does not include the following changes:

  • Movement penalties when wielding from inventory or when grabbed (7a6f938)
  • Revised fire menu including support for shoulder straps (44ddd13 and 4e77631)
  • Any new items beyond the above demonstration items

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

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()
@mugling mugling mentioned this pull request Nov 11, 2015
8 tasks
@chaosvolt
Copy link
Contributor

Hope it goes well.

@mugling
Copy link
Contributor Author

mugling commented Nov 11, 2015

@chaosvolt Thanks, its consumed a lot of time to date but I think we have finally reached the end

@chaosvolt
Copy link
Contributor

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
@kevingranade kevingranade merged commit c116bb1 into CleverRaven:master Nov 15, 2015
@mugling mugling deleted the iuse_holster_p1 branch November 24, 2015 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot cancel quickdraw from holster (4b8c3de)
3 participants