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

feat(content): Allow multiple required bionics #5496

Merged

Conversation

David-Silva-Jr
Copy link
Contributor

Checklist

Required

Optional

  • This is a C++ PR that modifies JSON loading or behavior.
    • I have documented the changes in the appropriate location in the doc/ folder.

Purpose of change

Add on to #5484 by allowing multiple bionics to be required to install a new bionic.

Describe the solution

Essentially the same as Zlorthishen's solution, just edited to also work with a list of required bionics.

Describe alternatives you've considered

Leave it the way it is.

Testing

1.) Create a world and character
2.) Give the character debug bionic installation
3.) Spawn an autodoc and couch
4.) Spawn Hydraulic Muscles and Skeletal Bracing
5.) Try to install Muscles before Bracing, see warning popup
6.) Install Bracing, then Muscles, then try to uninstall bracing, see warning popup
7.) Uninstall Muscles, then Bracing, as normal

You can also create a test bionic that requires multiple bionics to check the new functionality. I here are the objects I used for that:

bionic json

{
    "id": "bio_multi_test",
    "type": "bionic",
    "name": { "str": "Required Bionic Test CBM" },
    "description": "Just checking if multiple bionics can be required to install one bionic.",
    "occupied_bodyparts": [ [ "torso", 4 ], [ "arm_l", 1 ], [ "arm_r", 1 ], [ "leg_l", 1 ], [ "leg_r", 1 ] ],
    "flags": [ "BIONIC_FAULTY" ],
    "required_bionics": [ "bio_ankles", "bio_voice" ]
}

bionic item json

{
    "id": "bio_multi_test",
    "copy-from": "bionic_general_faulty",
    "type": "BIONIC_ITEM",
    "name": { "str_sp": "Required Bionic Test CBM" },
    "description": "Just checking if multiple bionics can be required to install one bionic..",
    "weight": "100 g",
    "difficulty": 1
}

Additional context

image
image
image
image

@github-actions github-actions bot added docs PRs releated to docs page src changes related to source code. JSON related to game datas in JSON format. labels Oct 3, 2024
Copy link
Contributor

autofix-ci bot commented Oct 3, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

chaosvolt
chaosvolt previously approved these changes Oct 3, 2024
Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

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

  1. Compiled and load-tested.
  2. Started as an industrial cyborg, no odd behavior found.

@chaosvolt
Copy link
Member

Error: /home/runner/work/Cataclysm-BN/Cataclysm-BN/src/bionics.cpp:414:9: error: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty,-warnings-as-errors]
    if( required_bionics.size() > 0 ) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
        !required_bionics.empty()
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:1083:7: note: method 'vector'::empty() defined here
      empty() const _GLIBCXX_NOEXCEPT
      ^
Error: /home/runner/work/Cataclysm-BN/Cataclysm-BN/src/bionics.cpp:2061:21: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
        std::string concatenated_list_of_dependent_bionics = "";
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                    concatenated_list_of_dependent_bionics
Error: /home/runner/work/Cataclysm-BN/Cataclysm-BN/src/bionics.cpp:2317:21: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
        std::string list_of_missing_required_bionics = "";
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                    list_of_missing_required_bionics
Error: /home/runner/work/Cataclysm-BN/Cataclysm-BN/src/bionics.cpp:2326:13: error: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty,-warnings-as-errors]
        if( list_of_missing_required_bionics != "" ) {
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !list_of_missing_required_bionics.empty()
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:1191:7: note: method 'basic_string<char>'::empty() defined here
      empty() const _GLIBCXX_NOEXCEPT
      ^

hecc

@David-Silva-Jr
Copy link
Contributor Author

I knew there'd be some weird string stuff, I'll go fix that

Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

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

Tests pass now, compiled and load-tested as before and still works as expected. Also loaded up and tested that it won't let me install the muscle CBM without augmenting my skeleton.

@chaosvolt chaosvolt merged commit 0039f73 into cataclysmbnteam:main Oct 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PRs releated to docs page JSON related to game datas in JSON format. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants