Jsonize vehicle parts requiring other vehicle parts to be installed #37453
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
Qrox
reviewed
Jan 28, 2020
BevapDin
reviewed
Jan 28, 2020
BevapDin
reviewed
Jan 28, 2020
src/vehicle.cpp
Outdated
| // Check all the flags of the part to see if they require other flags | ||
| // If other flags are required check if those flags are present | ||
| for( const std::string flag : part.get_flags() ) { | ||
| if( json_flag::get( flag ).requires_flag() != "" ) { |
There was a problem hiding this comment.
Suggested change
| if( json_flag::get( flag ).requires_flag() != "" ) { | |
| if( !json_flag::get( flag ).requires_flag().empty() ) { |
But better would be to change requires_flag_ to a cata::optional<std::string>.
BevapDin
reviewed
Jan 28, 2020
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: Jianxiang Wang (王健翔) <[email protected]>
Co-Authored-By: BevapDin <[email protected]>
Co-Authored-By: BevapDin <[email protected]>
Co-Authored-By: BevapDin <[email protected]>
I-am-Erk
added
[C++]
This was referenced Feb 9, 2020
…lio/Cataclysm-DDA into Hirmuolio-vehicle-jsonize
This was referenced Feb 16, 2020
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.
Summary
SUMMARY: Infrastructure "Jsonize vehicle parts requiring other vehicle parts"Purpose of change
Some vehicle parts require other vehicle parts before they can be installed. For example to install turret on a vehicle the tile must already have a turret mount.
This is controlled by flags of which the effect is hardcoded.
Install and uninstall requirements are also defined separately in different places leading to small bugs where install and uninstall requirements are different.
Describe the solution
The flags in json have a new property
requires_flag.If the flag has
requires_flagproperty then this flag means that this part requires that flag to be installed.If another part is blocking the removal of a part due to it requiring that another part you will see a "Remove the attached [part name]" message.
If multiple parts are blocking the removal only one of them is displayed.
This system will also work for parts that require more than one part. Just add multiple flags on the vehicle part.
Describe alternatives you've considered
Some of the "this part does something" flags could be separated from "this part requires that other part" flag. For example
ALTERNATORflag shouldn't require engine, instead the alternator part should have separateNEEDS_E_ENGINEflag for the part requirement.requires_flagis generic name but the system applies to only vehicle parts. The same property could probably be expanded to other things (gunmods, cloth modifications, buildings).It is possible to destroy the vehicle a vehicle part that is required by another vehicle part without destroying the other vehicle part. For example you can destroy the wheel hub withotu destroying the wheel that is connected on it. The connected part should fall off in these cases but that won't be done here.
Testing
Seems to work just fine.
Additional context