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

Cable rework part 2: Extension cords and more #66871

Merged
merged 69 commits into from
Aug 1, 2023

Conversation

Kamayana
Copy link
Contributor

@Kamayana Kamayana commented Jul 13, 2023

Summary

Features "Cable rework part 2: Adds extending cables with extension cords; adds disconnecting cables from examined vehicle part; improves cable status in item name; other cable improvements"

Purpose of change

I didn't implement everything I'd planned for cables in #64334, and there were some issues with the system that came to light after seeing it in action for a few months. This PR is a followup to improve the cable system further.

Player-facing changes:
  1. Cables and devices can now be extended by extension cords with the "can_extend" parameter in their link_up action. This adds the extension's length to the cable/device, letting it connect from further away. You can keep attaching more cables to make it even longer, and there is no limit to this, though the efficiency of all cables is multiplied together so the more cables attached, the less efficient it gets. Extended cables can be used to connect vehicles as well - you can daisy chain jumper cables to connect vehicles that are further apart, for instance.
  2. Linked up items now show more information in inventory menus: the length, max length, and number of extensions, all color coded to how much length is remaining. Unconnected, extended cables show a - as the current length, unspooled cables show a ×. Cables consolidate based on their length and status.
    image
  3. The "unplug power connections" option when examining appliances has been expanded to be usable on vehicle parts as well, though with vehicles you will only disconnect cables attached to the specific tile you're examining. There are also two separate actions for removing cables and power cables now, since we wouldn't want to disconnect a battery from the entire power grid just to unplug a laptop. The default hotkey for this action is c, with the "Fill container with water from faucet" action's hotkey being changed to capital F.
  4. The "stretched to its limit!" warning will now work better with circular distances. Before, it was possible to break a cable with zero warning, but now you should always get one unless you're moving multiple squares a turn.
  5. After detaching/re-spooling a cable, the menu will reopen for convenience, since you will generally only manually detach/respool a cable when you want to reconnect it to something.
  6. A cable will show its attached vehicle in the menu for attaching it, if any.
  7. JUMPSTART now needs a jumper cable specifically; a pile of meat doesn't have an outlet to plug an extension cord into.
  8. Fixes Plugged in tools cause crash when repairing #66944.
  9. Disconnecting vehicle cables will now place the cable right in the player's inventory if possible. This does not apply to removing cable parts through the vehicle menu, as that would have to be a more general change for all vehicle part interactions.

Describe the solution

Back-end changes:
  1. Electrical devices now contain their link_data directly, rather than creating an invisible cable item in their CABLE pocket to carry the link_data. This greatly simplifies how they're handled, and the CABLE pocket has been re-worked as the container for cable extensions. This does mean that the "generic_device_cable" item has to be deprecated, so I added an item migration that will turn them into oxygen that'll sit silently inside the CABLE pocket indefinitely, getting deleted if the item ever removes any cable extensions. I included a CABLE_SPOOL flag check in the cables() function to prevent cabled items from erroneously showing that oxygen as a cable extension, but that flag check could be safely removed eventually since there's no other way for a non-cable to get added to that pocket than this item migration.
  2. When processing a cable that's attached to a vehicle that's out of bounds, the game now loads up the vehicle's submap to assign the cable's vehicle pointer right away, rather than fudge the numbers until the vehicle is loaded by entering the reality bubble. This was accomplished by making the find_vehicle() function public.
  3. A new activity invoke_item_activity_actor was added, which is a simple activity that invokes a specific item, and optionally a specific method, and ends the activity.
  4. link_up actions now use Plug in / Manage cables as the default menu text, and use an ammo_scale of 0 by default. This lets json for link_up actions be much more concise.
  5. item_contents::clear_pockets_if was added, a function that lets you easily clear an item's pockets if they meet a chosen lambda function.
  6. Fixed a bug that made cable vehicle parts break off the turn AFTER they should have. Fixing this revealed that the power_cable_stretch_disconnect test was relying on that bug to return correct results, so I had to adjust things there as well.
  7. remove_remote_part() reworked, now there's a get_remote_part() function that returns both the remote vehicle and the part as a pair. remove_remote_part() now uses that new function to delete the part.
  8. find_vehicle() was made public, an easy way to load a vehicle outside the reality bubble into memory.
  9. properties_to_item() was renamed to part_to_item() and moved from being a vehicle_part function to a vehicle one, as it now needs to know the absolute position of the part, which the vehicle keeps track of. All uses of it were replaced appropriately.
  10. shed_loose_parts() was heavily reworked, as it didn't actually work properly for cables attached to a mount other than [0,0]. It's now far more robust, accurately updating connection points between vehicles when either of them moves or rotates, at any speed, and even beyond reality bubble boundaries.

Describe alternatives you've considered

  1. Apart from the efficiency loss, there's no downside to connecting different kinds of cables together. In real life, connecting cables of different gauges/amperage ratings is dangerous and will probably destroy the cords eventually, or even start a fire. It would be possible to simulate such things, but for this PR I mostly just wanted to make a suitable replacement for the vehicle connection daisy-chaining people were doing to connect disparate electric grids.
  2. The new disconnect cable option on the vehicle interaction menu uses the c hotkey, but when interacting with an appliance it will use the fallback hotkey assignment, which will probably assign it to a. veh_app_interact::populate_app_actions() has a set of specific commands it assigns hotkeys to, but assigns fallback alphabetical keys to everything else. Now that unplugging cables works for both vehicles and appliances, it can't be included in that list of specific commands anymore, and thus gets a fallback hotkey. I was able to make a fix for this, but it was hacky and felt like taking a shotgun to an issue that needs more nuance, so I ultimately let it be.
  3. When reopening the cable interaction menu, sometimes the progress menu will be stuck at the top of the screen. This seems to be an issue related to actions that are based on speed instead of time, and fixing it would be out of scope.

Testing

Device Cables Stress Test-trimmed.tar.gz
A helicopter with many, many cables attached, used for testing that everything stays connected even when flying at high speeds, and stress testing to see the CPU impact. The results after this PR were much the same as the results for the first cable PR.

Device Cables All Connections-trimmed.tar.gz
A save created in a recent experimental version (e284941) with every kind of cable connection active. Used to ensure this PR's changes can handle saves made after #64334. Vehicle connections and cable items stay linked, but electrical devices do not. This was an expected outcome, and any attempt to somehow migrate the links wouldn't be worth it in my opinion, since players could easily just re-link the devices.

Additional context

This PR is in draft so after #66847 gets merged I can adjust it accordingly. My sincere thanks to everyone who added link_up actions to items, something I said I would do myself before getting lost in the weeds of making this PR. I made sure to not invalidate their work.

@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON Vehicles Vehicles, parts, mechanics & interactions Items: Battery / UPS Electric power management Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` [Markdown] Markdown issues and PRs Items: Containers Things that hold other things Items: Armor / Clothing Armor and clothing Appliance/Power Grid Anything to do with appliances and power grid <Enhancement / Feature> New features, or enhancements on existing labels Jul 13, 2023
data/json/items/tool_armor.json Outdated Show resolved Hide resolved
data/json/items/tool_armor.json Outdated Show resolved Hide resolved
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Jul 13, 2023
@Nebnis
Copy link
Contributor

Nebnis commented Jul 14, 2023

This was an amazing and very much needed PR for the electrical grid thank you very much for doing it :). It will make doing static bases a lot more fun now

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 14, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Auto-requesting reviews from non-collaborators: @Qrox

@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 17, 2023
data/raw/keybindings/vehicle.json Outdated Show resolved Hide resolved
src/iuse_actor.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 19, 2023
@bombasticSlacks
Copy link
Contributor

Just reviewed the code and everything LGTM one conflict to be resolved and then I'm happy to merge :)

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 29, 2023
@Maleclypse
Copy link
Member

Linked items now sort by link status first, then by maximum length, then by current length from shortest to longest:

image

Is this intended to still be in draft or is it ready for merge?

@Kamayana
Copy link
Contributor Author

Kamayana commented Jul 31, 2023

Is this intended to still be in draft or is it ready for merge?

It's intended, there's still more incoming. It started as just moving find_vehicle back to a static vehicle function as andrei8l suggested but it snowballed into more significant rewrites as I discovered out-of-bubble vehicle connections didn't actually work for anything but appliances. Nearing completion on that, though, and then I can undraft this.

@Kamayana
Copy link
Contributor Author

Kamayana commented Jul 31, 2023

Significant reworks here:

  1. Finding the maximum length of an unconnected, extended cable used to be laborious, having to fetch the link_up iuse actor data, then iterate through all the contained cables and adding their lengths. It's now much more efficient, as it keeps the link_data active for any item with extensions and just returns link->max_length.
  2. remove_remote_part() reworked, now there's a get_remote_part() function that returns both the remote vehicle and the part as a pair. remove_remote_part() now uses that new function to delete the part.
  3. find_vehicle() was switched back from map to a static vehicle function, though it's now public.
  4. properties_to_item() was renamed to part_to_item() and moved from being a vehicle_part function to a vehicle one, as it now needs to know the absolute position of the part, which the vehicle keeps track of. All uses of it were replaced appropriately.
  5. shed_loose_parts() was heavily reworked, as it didn't actually work properly for cables attached to a mount other than [0,0]. It's now far more robust, accurately updating connection points between vehicles when either of them moves or rotates, at any speed, and even beyond reality bubble boundaries.
  6. Disconnecting cables will now place the cable right in the player's inventory if possible. This does not apply to removing cable parts through the vehicle menu, as that would have to be a more general change for all vehicle part interactions.

I think this is ready now, assuming the tests are green. Probably needs another round of reviews on the new changes, though.
Edit: they were not green, welp. I need to sleep, I'll look into it tomorrow.
Edit2: Oh shoot and I just remembered I stashed some other changes I forgot to make into a proper commit, so don't move ahead with this yet.

@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 31, 2023
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Aug 1, 2023
@Kamayana Kamayana marked this pull request as ready for review August 1, 2023 11:12
@Maleclypse Maleclypse merged commit e6f7407 into CleverRaven:master Aug 1, 2023
@Kamayana Kamayana mentioned this pull request Aug 2, 2023
@Kamayana Kamayana deleted the cables_part_2 branch January 9, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Appliance/Power Grid Anything to do with appliances and power grid 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. <Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. Items: Armor / Clothing Armor and clothing Items: Battery / UPS Electric power management Items: Containers Things that hold other things [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugged in tools cause crash when repairing
6 participants