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

Matchbox fix #39551

Merged
merged 3 commits into from
Apr 14, 2020
Merged

Matchbox fix #39551

merged 3 commits into from
Apr 14, 2020

Conversation

kevingranade
Copy link
Member

Summary

SUMMARY: None

Purpose of change

Fixes #39043
We've had a very intermittent error for who knows how long when matchbooks are spawned.
It turns out this happens when an item_group has a non-zero with_ammo field and a child item modifier of it is a tool with default charges and a null ammo type.

Describe the solution

The issue turns out to be a mismatch between null strings produced by item::default_ammo() and expected by item::ammo_set().

Describe alternatives you've considered

A more complete fix to this would be to make itype_id or possibly just the ammo string a formal type instead of using std::string, but that's a pretty huge change I don't have the stomach to tackle right now.
A slightly more complete fix that I might consider soon is auditing the uses of "null" vs "NULL" in the item code and data to eliminate errors of this kind, but we need to be careful because remaining mismatches can cause further errors.

Testing

The recently added mx_minefield test was triggering this fairly frequently by simply generating a map extra that contained an affected item group. I was able to isolate the cause and add a unit test for it.

@ZhilkinSerg ZhilkinSerg added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves labels Apr 14, 2020
@ZhilkinSerg ZhilkinSerg merged commit a95b583 into CleverRaven:master Apr 14, 2020
@kevingranade kevingranade deleted the matchbox-fix branch June 28, 2020 21:13
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) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tried to set invalid ammo of undefined-NULL for matchbook
2 participants