Refactor item templates [RDY] #15908
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Rebased against master |
| @@ -589,6 +588,10 @@ void Item_factory::check_definitions() const | |||
| if( type->item_tags.count( "BIO_WEAPON" ) ) { | |||
| msg << "BIO_WEAPON must not be specified with an ammo type" << "\n"; | |||
| } | |||
|
|
|||
| if( !type->magazines.empty() && !type->magazine_default.count( type->gun->ammo ) ) { | |||
| msg << "specified magazine but none provided for default ammo type" << "\n"; | |||
There was a problem hiding this comment.
Causes errors with mods, specifically Generic Guns.
There was a problem hiding this comment.
I'll take a look now
|
Worse: it seems to cause a segfault with the no magazines mod. |
I have debug symbols for that build so I'll take a look. |
|
Almost certainly 90db4e1 but I'm struggling to see why |
|
Maybe you're checking the magazine field of a gun that no longer has one? |
|
I think I have it, the conditional for |
|
be84b8c fixes the segfault but the generic guns mod is still unhappy |
|
Have candidate fix for generic guns, waiting on |
|
Fixed in 387bf07 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were only two instances where
itypetemplates were modified after loading. The first was the charge rifle (dropped in #15713) and the second in defense mode where for some reason2x4is set to have zeroweightandvolume(dropped in 6b7a8dd).This PR refactors
Item_factory::find_templateto returnconst itype *and changes the storage of template to be backed bystd::unique_ptr. Item templates are now effectively immutable after loading although new templates can still be added. Nothing prevents the later addition of an explicitmutate_templatemethod if such a need arises.Incidentally noted during refactoring the parsing of
magazineis insufficiently strict and I suspectnullptrreferences are possible dependent upon theJSONdata. This is fixed in c057f35 which adds an additional test toItem_factory::check_definitions()Item_factory::get_all_itypes()questionably breaks encapsulation so a future PR could consider deprecating that also.Trivial merge conflict with #15901 should be easily resolvable when merge testing.