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

No magazines mod #15490

Merged
merged 4 commits into from Feb 21, 2016

Conversation

Projects
None yet
5 participants
@kevingranade
Copy link
Member

commented Feb 20, 2016

Turns out this was pretty easy to do, so why not?

Reverts all guns to use integral magazines and blacklists all magazine items.
Integral magazine capacity is based on the default magazine for the gun.

kevingranade added some commits Feb 19, 2016

Add infrastructure for a feature that reverts guns to all use integra…
…l magazines for simpler gameplay and a mod that triggers it.
@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2016

Ah, interesting to see. This might also render the old magazine mods useful again, as mod content. Or at least used in such a mod.

}

Item_factory::~Item_factory()
{
item_options.clear();

This comment has been minimized.

Copy link
@BevapDin

BevapDin Feb 20, 2016

Contributor

This should probably be in Item_factory::clear. The factory is only destroyed when the game is terminated, but clear is called before loading the game data.

Btw. why not make item_options a member or at least put it into an anonymous namespace/mark it as static?

This comment has been minimized.

Copy link
@kevingranade

kevingranade Feb 20, 2016

Author Member

Yes to both, thanks.

// deleted before the guns are processed.
if( magazines_blacklisted ) {
for( std::map<std::string, itype *>::const_iterator a = m_templates.begin();
a != m_templates.end(); ++a ) {

This comment has been minimized.

Copy link
@BevapDin

BevapDin Feb 20, 2016

Contributor

Why not use a range-based loop? Same question for the loop below.

This comment has been minimized.

Copy link
@kevingranade

kevingranade Feb 20, 2016

Author Member

Excuse my laziness, I was just copying, will fix soon.

}
itype *default_magazine = m_templates[ type->magazine_default.begin()->second ];
type->gun->clip = default_magazine->magazine->capacity;
type->gun->reload_time = default_magazine->magazine->reload_time;

This comment has been minimized.

Copy link
@mugling

mugling Feb 20, 2016

Contributor

reload_time for magazines is per round so need to multiply by magazine capacity

@mugling

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2016

Turns out this was pretty easy to do, so why not?

Given you don't have to touch the reloading code to do this and we intend to continue supporting integral magazines in mainline this is an excellent solution.

This might also render the old magazine mods useful again

The gunmod code is horrible and still has a lot of outstanding bugs. The required work to reimplement this isn't presently worth the outcome.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2016

The gunmod code is horrible and still has a lot of outstanding bugs. The required work to reimplement this isn't presently worth the outcome.

Hmm. Understandable. If we are able to support the old magazine gunmods without additional hassle then it would be useful, but if it proves to complicate things, then it's fine to leave them dummied-out.

@Coolthulhu Coolthulhu self-assigned this Feb 21, 2016

Coolthulhu added a commit that referenced this pull request Feb 21, 2016

@Coolthulhu Coolthulhu merged commit 60dd09f into CleverRaven:master Feb 21, 2016

1 check passed

default This has been rescheduled for testing as the 'master' branch has been updated.

@kevingranade kevingranade deleted the kevingranade:no-magazines-mod branch Apr 30, 2018

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.