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

Fix UPS guns for multiple ammotype support #31442

Merged

Conversation

mark7
Copy link
Contributor

@mark7 mark7 commented Jun 15, 2019

Recent support for multiple ammotypes broke UPS guns, as they have no
ammotype. Fix.

Summary

SUMMARY: Bugfixes "Fix electrically-powered vehicle guns broken by multiple ammotype support"

Purpose of change

Fixes #31399

Recent support added for multiple ammotypes in hashref 64516a6 had caused character creation to bail out when using mods containing electrically-powered vehicle weapons with no ammotype. This included CrazyCataclysm and Aftershock.

Describe the solution

Doesn't insert ammo into guns that have no ammotype.

@mark7
Copy link
Contributor Author

mark7 commented Jun 15, 2019

This patch seems to me to work in a quick test (and gets master working with Aftershock and Crazy Cataclysm again), but (a) I have no familiarity with this portion of the code, and (b) watching the changelogs, the design of things related to charges seem to be in flux at the moment. I'd appreciate a glance from someone familiar with how vehicle weapons ought to work (@ymber, if you're handy?). Thanks.

src/veh_type.cpp Outdated
@@ -1061,7 +1061,9 @@ void vehicle_prototype::finalize()
}
}
if( pt.ammo_types.empty() ) {
pt.ammo_types.insert( ammotype( *base->gun->ammo.begin() )->default_ammotype() );
if (!base->gun->ammo.empty()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!base->gun->ammo.empty()) {
if( !base->gun->ammo.empty() ) {

Recent support for multiple ammotypes broke UPS guns, as they have no
ammotype.  Fix.
@mark7 mark7 force-pushed the wb-fix-ups-guns-multi-ammotype branch from 47ed86d to 9f6e052 Compare June 15, 2019 04:54
@ZhilkinSerg ZhilkinSerg added <Bugfix> This is a fix for a bug (or closes open issue) Vehicles Vehicles, parts, mechanics & interactions labels Jun 15, 2019
@ghost ghost mentioned this pull request Jun 15, 2019
@ZhilkinSerg ZhilkinSerg merged commit f52e052 into CleverRaven:master Jun 15, 2019
@ymber
Copy link
Member

ymber commented Jun 15, 2019

@mark7 This is how vehicle weapons ought to work. Checking the prototype instead of the base item was an oversight in #30874.

@mark7 mark7 deleted the wb-fix-ups-guns-multi-ammotype branch June 25, 2019 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Character creation crash
4 participants