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

Bionics can require another bionic #66996

Merged

Conversation

anoobindisguise
Copy link
Contributor

Summary

Content "bionics can require another bionic"

Purpose of change

Some bionics didn't make sense to be installed without other bionics present. I wanted to change that, but the existing "upgrades" structure for bionics is for replacing bionics with new versions, rather than installing them side by side.
This will probably help us do "framework" bionics where you can install certain bionics like a bionic arm, and then use that as a framework for for example the bionic monoblade or shotgun.

Describe the solution

You can now specify exactly one bionic that is needed to be installed in order to install a specific bionic.
As an example of the system, I made Hydraulic Muscles require Titanium Skeletal Bracing, as I think this one would be pretty uncontroversial (you're so strong under hydraulic muscles you would probably break your own bones)

Describe alternatives you've considered

I've seen some concern that such a system might not be beneficial to the game - bionics are designed to be "plug and play" and making too many bionics require other bionics will reduce build variety by forcing everyone to get specific "base" bionics in order to get the ones they want. However I don't really agree with that and think that as long as this system isn't used excessively bionics can progress in a "tree" which still offers plenty of variety. I also hope that this system will allow the addition of new bionics which we couldn't have before because there was no way to ensure you had a secondary bionic which would be necessary to make it function.

I debated allowing multiple requirements but I think this is better, as it reads more nicely than a list of different bionics, and is easier for players, and won't shoehorn too many bionics into chaining together and reducing build variety.

Testing

Compiled, tested, confirmed functional.

Additional context

The bionic that has a missing prereq shows up as gray in the inventory rather than the usual green. You can also see in the item description which bionic is needed as a prereq. The bionic install menu shows the reason it can't be installed. After installing the prereq, it shows as being green in inventory because its prereq is met, and it can be installed. Also, the required cbm cannot be uninstalled while the cbm is installed, but once it is removed it can also be removed again.

reqbionic1
reqbionic5
reqbionic2
reqbionic3
reqbionic4

anoobindisguise and others added 13 commits July 15, 2023 14:51
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Bionics CBM (Compact Bionic Modules) [C++] Changes (can be) made in C++. Previously named `Code` labels Jul 18, 2023
src/bionics.cpp Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jul 18, 2023
@worm-girl
Copy link
Contributor

worm-girl commented Jul 18, 2023

This is a bad idea and moreover a bad precedent. We are always going to have bionics which are more desirable than others and hooking them into a system of prereqs means that there is going to be a substantial set of bionics which are seen as necessary just because the good stuff is gated behind them. This will be an annoyance at first. The CBM list is huge and finding a rare item only to be told that you can't use it until you find a second needle in a haystack is going to rub people the wrong way. Once bionic slots are mainlined however it's going to be a real problem.

This is the kind of thing that creates cookie cutter builds. The beauty of the existing mutation and bionic systems are that you have a lot of freedom to come up with intetesting loadouts, but there's been a creeping trend to curb that freedom. While some of it makes sense, like joint servos being incompatible with octopus tentacles, changes like this one seem to be monkeying with the system just for the sake of doing it. The result is more cost than benefit.

The hydraulic muscles CBM was not envisioned as something that would require a special skeleton to work. That's an invented justification strictly for the sake of this PR. While it's a minor enough change, it creates a precedent that will prompt further drift in this direction as people's natural tendency to want to sort things into subcategories kicks in.

@anoobindisguise anoobindisguise marked this pull request as draft July 18, 2023 19:40
@Procyonae
Copy link
Contributor

As a single level do you have X I see this as good but turning this into "trees" by nesting this behaviour in the future as you've suggested sounds awful, even more so while CBMs are limited primarily to Rubik

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

anoobindisguise commented Jul 18, 2023

Rubik being the primary source of CBMs is definitely a problem for this PR. Quite frankly they suck and getting a set of bionics you want requires several weeks of going back and forth between their shop even if you savescum heavily to get them to sell you the bionics you want (which by default, they usually won't). I can see how needing Bionic X to install Bionic Y would be really goddamn annoying if the primary source of CBMs spends several months deciding not to sell you Bionic X due to bad RNG. Rubik being a terrible vendor is why you have to dive dozens of microlabs to get the set you want and it's one of the worst parts of Cata gameplay right now IMO.

@worm-girl
Copy link
Contributor

worm-girl commented Jul 18, 2023

Is that really only a "right now" problem though? I don't think there are plans to make CBMs commonly available. There might be more zomborg locations added as bionics get migrated out of labs, but that's not really going to substantially change the rate the player comes across them is it?

@Procyonae
Copy link
Contributor

Is that really only a "right now" problem though? I don't think tgere are plans to make CBMs commonly available. There might be more zomborg locations added as they get migrated out of labs, but that's not really going to substantially change the rate the player comes across them is it?

I asked about adding zomborg locations and lore peeps said that wasn't even wanted 🤷

@github-actions github-actions bot added the <Documentation> Design documents, internal info, guides and help. label Jul 18, 2023
@github-actions github-actions bot added the [Markdown] Markdown issues and PRs label Jul 18, 2023
@github-actions github-actions bot removed json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 19, 2023
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@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 19, 2023
@anoobindisguise
Copy link
Contributor Author

I've gone ahead and removed the requirement of hydraulic muscles needing titanium skeletal bracing for now. The question of whether that should happen can wait until later, for now I know the framework is functional but I want to just introduce it alone without any other changes.

@anoobindisguise anoobindisguise marked this pull request as ready for review July 19, 2023 04:21
@Maleclypse
Copy link
Member

Is that really only a "right now" problem though? I don't think tgere are plans to make CBMs commonly available. There might be more zomborg locations added as they get migrated out of labs, but that's not really going to substantially change the rate the player comes across them is it?

I asked about adding zomborg locations and lore peeps said that wasn't even wanted 🤷

It's not wanted because Zomborgs aren't the desired state for advanced cbm drops. What are desired are specific high difficulty missions from Rubik with the ability to choose one of two advanced CBMs from as a reward and Labyrinthine structures which would be mutable mapgen points that are basically dungeons into Nether locations that have weird/advanced cbms in them.

@Procyonae
Copy link
Contributor

It's not wanted because Zomborgs aren't the desired state for advanced cbm drops. What are desired are specific high difficulty missions from Rubik with the ability to choose one of two advanced CBMs from as a reward and Labyrinthine structures which would be mutable mapgen points that are basically dungeons into Nether locations that have weird/advanced cbms in them.

I'm kind of into the idea of just unhardcoding and improving as much existing mapgen as possible rn but I'd happily make a map like that at some point if someone else comes up with suitable lore with an outline of the quest structure

@Maleclypse Maleclypse merged commit bc2cb58 into CleverRaven:master Jul 22, 2023
Zlorthishen added a commit to Zlorthishen/Cataclysm-BN that referenced this pull request Oct 1, 2024
https: //github.com/CleverRaven/Cataclysm-DDA/pull/66996
Co-Authored-By: anoobindisguise <[email protected]>
chaosvolt added a commit to cataclysmbnteam/Cataclysm-BN that referenced this pull request Oct 2, 2024
* initial commit

https: //github.com/CleverRaven/Cataclysm-DDA/pull/66996
Co-Authored-By: anoobindisguise <[email protected]>

* style(autofix.ci): automated formatting

* Update bionics.cpp

* style(autofix.ci): automated formatting

* Update bionics.cpp

* Update json_info.md

* style(autofix.ci): automated formatting

* Update doc/src/content/docs/en/mod/json/reference/json_info.md

* style(autofix.ci): automated formatting

* Woe, profession update upon thee

---------

Co-authored-by: anoobindisguise <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: scarf <[email protected]>
Co-authored-by: Chaosvolt <[email protected]>
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 Bionics CBM (Compact Bionic Modules) [C++] Changes (can be) made in C++. Previously named `Code` <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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants