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

vehicles: simplify specifying directional variants of vehicle parts #42803

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

mlangsdorf
Copy link
Contributor

@mlangsdorf mlangsdorf commented Aug 8, 2020

Summary

SUMMARY: Infrastructure "vehicles: simplify specifying directional variants of vehicle parts"

Purpose of change

Currently vehicle parts with directional variations (like the NE and NW corner frames) are specified as separate parts, usually as copy-from of an abstract. This needlessly adds noise to the JSON and complicates the code, because deciding frame_nw and frame_ne are the same requires messing with breaking up the vpart_ids.

Simplify the code and cut away a lot of JSON by adding the concept of a variant. A vpart_info with a variant just has different symbols/sprites, depending on which variant of the part is installed, but all of them have the same vpart_info.

A set of standard directional variants means that existing mods and vehicle designs are not affected, and the syntax for specifying a vehicle part is unchanged. { "x": 0, "y": 0, "part": "frame_cross" } still spawns a cross frame part at 0,0, even if internally it's a little different.

Describe the solution

Add a new optional field to vpart_info "symbols", which is a map of direction strings to symbol ints.

Add a new optional field to vehicle_part "variant", which is a string that contains the directional variant.

Add a lot of complicated parsing to load existing save game and JSON data files into variant vehicle parts. Vehicle parts that use the 10 standard direction variants with their standard symbols can replace "sym" with "standard_symbol": true and have those automatically loaded; other parts will need to specify the variant string to symbol mapping as an object.

vehicle_display.cpp returns the appropriate symbol for the specific variant of a vehicle part, or alternately for cata_tiles, returns the old style vehicle part ID, ie "frame_cross" for the cross variant of a frame.

The vehicle interaction menu displays names for the part variants. They're currently hardcoded and translated. I welcome suggestions for better names.

Documentation has been updated to take into account variants.

Describe alternatives you've considered

Leave things as they are, but that will get crazy as i-am-erk adds more vehicle part rotations.

Testing

Passes unit tests. I can spawn a bus and install the various variant parts on it, save and load it, and get the same bus back. I can also load existing saves without causing visual glitches.

Additional context

Will add screenshots shortly

@mlangsdorf mlangsdorf added SDL: Tiles / Sound Tiles visual interface and sounds. Vehicles Vehicles, parts, mechanics & interactions labels Aug 8, 2020
@mlangsdorf mlangsdorf force-pushed the vpart_simplified branch 2 times, most recently from 788693c to 7a4548a Compare August 10, 2020 14:00
@I-am-Erk
Copy link
Member

I can send you the vp IDs or just edit them in myself if it makes your life easier.

@mlangsdorf mlangsdorf force-pushed the vpart_simplified branch 6 times, most recently from e17f2b3 to 9990681 Compare August 13, 2020 16:52
@mlangsdorf mlangsdorf marked this pull request as ready for review August 13, 2020 16:55
doc/JSON_INFO.md Outdated Show resolved Hide resolved
src/veh_type.h Show resolved Hide resolved
src/veh_type.h Show resolved Hide resolved
Copy link
Member

@I-am-Erk I-am-Erk left a comment

Choose a reason for hiding this comment

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

Looking through this seems to be about everything missing for my about-to-add parts to join the club once this is in. Need to do a quick find-and-replace in vehicles.json for the door style name changes, do you want me to do that?

src/veh_type.h Outdated Show resolved Hide resolved
src/veh_type.h Outdated Show resolved Hide resolved
src/veh_type.h Outdated Show resolved Hide resolved
instead of having 8-9 different vehicle part JSON definitions for
boards, quarterpanels, and frames that differ only by id and symbol,
have a single vehicle part JSON definition that has multiple symbols.

Make this transparent for mods by parsing the vehicle part id for the
standard variants (cover, cross, horizontal, etc).
…suals

Convert the standard "bus" vehicle to use "parts" arrays instead of
long lists of "part", and use some of the new directional part variants to
make the bus look better under tiles.
@ZhilkinSerg ZhilkinSerg merged commit c73a6f0 into CleverRaven:master Aug 18, 2020
@mlangsdorf mlangsdorf deleted the vpart_simplified branch August 18, 2020 12:07
@kevingranade
Copy link
Member

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/start-vehicle-construction-now-requires-specific-frames-to-start-version-0-e-5175-g3f9c144-build-10923/24489/2

kevingranade pushed a commit that referenced this pull request Aug 19, 2020
Added missing INITIAL_PART to all the frames which were missed by the last change (PR #42803).
Replaced the only remaining abstract with id, as it's no longer in use as an abstract.
def.sym = symbol;
} else {
def.symbols[ variant_id ] = symbol;
}
def.sym = jo.get_string( "symbol" )[ 0 ];
Copy link
Contributor

@ZhilkinSerg ZhilkinSerg Oct 8, 2020

Choose a reason for hiding this comment

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

Looks like this line is redundant now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SDL: Tiles / Sound Tiles visual interface and sounds. Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants