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

Provide more ammo accessors #14837

Merged
merged 4 commits into from Jan 16, 2016

Conversation

Projects
None yet
3 participants
@mugling
Copy link
Contributor

commented Jan 13, 2016

  • Implements ammo_data() and ammo_current() from #14799 excluding handling of magazines
  • Deprecates get_curammo_id() which is trivially replaced by ammo_current()
  • Refactors item::weight() to use new accessors plus simplify logic and astyle

Further PR's will extend usage of these accessors and ultimately deprecate get_curammo. When we then merge the magazine type all functions using these accessors will be aware of magazines.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jan 13, 2016

I'm not sure if that is a step towards ammo overlap (see .38 Special plus .357 Magnum, .410 shotshells plus .45 Colt, etc) or something with different applications.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jan 13, 2016

No, its just the two accessor functions extracted from #14799. They are intended to replace get_curammo_id and eventually get_curammo.

I'm going to work sequentially through consumers of get_curammo and check there are no likely issues when magazines are added and then change them to use ammo_data. Each logical block of functionality can have it's own PR for ease of testing to ensure there are no regressions. Finally we then re-add the magazine code to ammo_data and we're good to go with #14799 or it's successor.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jan 13, 2016

That would be failure to read on my part, as the original post states that this is tied to the shift towards magazines, Doh. >.<

@kevingranade kevingranade merged commit 341bae8 into CleverRaven:master Jan 16, 2016

1 check passed

default
Details
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.