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

Migrate convertible to foldable #59572

Merged
merged 8 commits into from
Aug 17, 2022

Conversation

irwiss
Copy link
Contributor

@irwiss irwiss commented Jul 24, 2022

Summary

Bugfixes "Migrate convertible stuff to foldable"

Purpose of change

Fixes #48113
Fixes #46752
Fixes #28976
Fixes #39399
Fixes #51005

Describe the solution

Patch converts all "convertible" items and vehicles to their to "generic" foldable equivalents.

This gets rid of all convertible-related code, including vehicle install/remove blocking on any vehicle with convertible flag, with minimal losses (some things like #28976 are a write off in existing saves)

There was an earlier attempt in #46805 but I don't believe you can do this migration mostly lossless with json alone.

Had to add some code to support 2 things:

  1. Add "item_variables" to itype, they are then copied over to instance of item's item_vars when one is created - this is so itypes are able to store variables as "presets", once an item is spawed it gets a copy and is then taking over and using their normal item::get_var/set_var to manipulate them. This allows having a preset of "folding_parts" the generic unfolding can use to produce a vehicle out of that parts list.

  2. Add support for reset_item_vars in Item_factory::migrate_item , this allows to reset the folding_parts (or any other) variable to the item prototype value, this allows to migrate the old parts to new and add the corresponding folding_parts so they can unfold.

Stuff changed

  1. The folding_bicycle vehicle used light frames (so: not "generically" foldable), and "convertible" tag + iuse action that used convertible tag to spawn a fresh bicycle on activation, this caused a bunch of issues from the flag not persisting through racking on bike rack to repairing demolished tiles on unfolding.

  2. wheelchair the vehicle already uses folding parts therefore mostly cosmetic adjustments were done to it.

    • The item folded_wheelchair which used convertible code is migrated to folded_wheelchair_generic which is the generic one.
    • It now has a folded_wheelchair_generic item that can be placed and unfolds into the wheelchair vehicle.
    • Existing wheelchair vehicles are untouched, they should fold into item closely matching folded_wheelchair (sans vehicle name).
  3. Inflatable boat is probably weirdest of them all, it also had the same issues but also didn't sink if you destroyed one of the parts (sinking is out of scope of this patch)

The bad parts

  1. The "generic" folding code is saving json string as one of the item's variables, and that causes the prototypes for having an unfoldable item be super ugly since it's escaped json string as seen for example here can't think of a better solution right now that doesn't involve changing more code than necessary.

  2. Inflatable boat required a pump before - it was handled in the iuse handler, for now the "generic" folding vehicles have nowhere to put it into and I wanted to keep this already awkward patch relatively light - I cheated a bit by adding pump into the description. Folding/unfolding has to become an activity either way, as current time of 500 moves to un/fold is unreasonable, that's the opportunity to let specific parts (inflatable bags for example) require pumps to unfold, or other tools depending on the parts involved.

  3. Any folding bicycle that already passed a bike racking activity "lost" it's convertible flag and can no longer be differentiated from any other bicycle, so I decided to write them off and leave them as non-foldable.

Describe alternatives you've considered

Leaving convertible code to rot more

Testing

Before applying patch prepare map or load the one I prepared:
for each of (folding bicycle, wheelchair, inflatable boat):
spawn two and drop them nearby
spawn one and inflate/unfold it nearby
spawn one + any vehicle with rack and rack it on the vehicle

Save, apply patch and reload that save, check that:

  • folding bicycle item should convert to the new one and still unfolds when you activate it
  • folding bicycle item 2 should match the weight/volume of the one you unfolded after folding it.
  • folding bicycle vehicle should convert to the new one with folding frames
  • folding bicycle on vehicle rack is still using light frames and unfoldable if you take it off (due to Foldable bike can't be folded after unloaded from bike racks #28976)
  • wheelcart item should unfold into a wheelcart when you activate it
  • wheelcart item 2 should match the weight/volume of the one you unfolded after folding it.
  • wheelcart the vehicle should have no changes
  • wheelcart on vehicle rack should be folding after you take it off
  • inflatable boat item should unfold into a boat when you activate it
  • inflatable boat 2 should match the weight/volume of the one you unfolded after folding it.
  • inflatable boat the vehicle should have no changes
  • inflatable boat the vehicle should not allow you to install/remove sections or inflated bags.

Unit test is removed for 3 reasons:

  1. It misses basic things like demolishing whole tiles makes them regenerate on fold/unfold cycle (demonstrated in video 1)
  2. There is no longer an 'iuse' to actually run the check, or degradation "transfer" that the old code was trying to test for - the degradation is stored as part of the "folded_parts" on the item. Manual check with new code is in video 2. Vehicle part degradation in new foldable code is handled by general item serialization code.
  3. The current fold/unfold is subpar, it needs to be migrated to activity to allow interruptions and reasonable un/fold times and requiring tools, which will be a follow up patch.

Additional context

Regenerating parts of a vehicle on fold/unfold cycle (in old convertible code)
https://user-images.githubusercontent.com/6560075/180669156-1164c82e-72c8-4ecd-8400-d362010e0fb1.mp4

"New" folding vehicles keep their damage and degradation on fold/unfold cycle
https://user-images.githubusercontent.com/6560075/180669217-cc95cd99-9959-40e9-96e9-6998e6c3343c.mp4

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Code: Tests Measurement, self-control, statistics, balancing. Vehicles Vehicles, parts, mechanics & interactions <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies NPC / Factions NPCs, AI, Speech, Factions, Ownership Spawn Creatures, items, vehicles, locations appearing on map and removed json-styled JSON lint passed, label assigned by github actions labels Jul 24, 2022
@irwiss irwiss force-pushed the migrate-convertible-to-foldable branch from b6fe0b1 to 5712a69 Compare July 24, 2022 23:23
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 24, 2022
@irwiss irwiss marked this pull request as ready for review July 25, 2022 00:57
@irwiss irwiss force-pushed the migrate-convertible-to-foldable branch 4 times, most recently from edcff4c to 151721c Compare July 26, 2022 06:41
@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions json-styled JSON lint passed, label assigned by github actions and removed json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 26, 2022
@irwiss irwiss marked this pull request as draft July 28, 2022 04:06
@irwiss irwiss force-pushed the migrate-convertible-to-foldable branch from 151721c to 369276c Compare July 30, 2022 16:14
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [Markdown] Markdown issues and PRs BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 30, 2022
@irwiss irwiss marked this pull request as ready for review July 30, 2022 20:01
@irwiss irwiss force-pushed the migrate-convertible-to-foldable branch from 369276c to 5341c34 Compare August 12, 2022 11:15
@irwiss irwiss force-pushed the migrate-convertible-to-foldable branch from 5341c34 to 4229494 Compare August 13, 2022 20:30
@Fris0uman Fris0uman merged commit 646fa4d into CleverRaven:master Aug 17, 2022
@irwiss irwiss deleted the migrate-convertible-to-foldable branch August 17, 2022 09:49
Hirmuolio pushed a commit to Hirmuolio/Cataclysm-DDA that referenced this pull request Aug 27, 2022
* Allow itype definitions assign item's item_vars

* Add docs on item properties, variables, itype's variables

* Allow migrations to reset item's variables, document it

* Migrate saved vehicles to generic foldables

* Migrate folding bicycle, wheelchair, inflatable boat items

* Remove "convertible" and unfold iuse from C++ side

* Cosmetic changes

* Adjust item groups, professions to new folded items
chaosvolt added a commit to chaosvolt/MST_Extra_Mod that referenced this pull request Sep 18, 2022
So it turns out a long-forgotten side effect of CleverRaven/Cataclysm-DDA#21545 was breaking the janky hardcoded injection of certain flags and ammo effects into

cataclysmbnteam/Cataclysm-BN#1857 makes this more important in BN but implemented same general changes to both versions in case DDA ever notices that function (and it's been about 5 years so literally everyone evidently never noticed any changes to crossbow behavior).

Also updated the deployable vehicles in DDA version, as CleverRaven/Cataclysm-DDA#59572 updated folding vehicles. While a great idea on paper, in practice they somehow managed to invent a form of JSON that's even MORE of an unreadable mess than vehicle JSON is, with making folded_parts something you have to work with to implement deployable vehicles now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <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. <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies NPC / Factions NPCs, AI, Speech, Factions, Ownership Spawn Creatures, items, vehicles, locations appearing on map Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
2 participants