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: Add exclusions and exclusion categories to gunmods #4254

Merged
merged 5 commits into from
Mar 2, 2024

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Feb 25, 2024

Purpose of change

Allow further fine-tuning of what guns can and cannot be installed with specific gunmods, both for further development and specificity of vanilla gunmods and for the use of mod authors.

Describe the solution

Adds mod_exclusions and mod_exclusion_category as optional fields in gunmods. This allows contributors to add specific blacklists (say you want to make a gun mod installable on all rifles except the Rivtech battlerifle for instance), and blacklist categories (all rifles unless they are energy weapons).

Blacklists take priority over target categories but not target ids. Priority for inclusion/exclusion goes mod_targets > mod_exclusions > mod_exclusion_category > mod_target_category

Note that it will scream at you if you try to include and exclude the same id or the same set of categories.

Changes the gyroscopic stabilizing system as a proof of concept for this system, and to ensure it's used somewhere in the vanilla game to avoid it being lost to time.

Describe alternatives you've considered

Testing

  • Spawn an A7 laser rifle, a gyroscopic stabilizer and a modified gyroscopic stabilizer.
  • Check that modified gyroscopic stabilizer is installable but gyroscopic stabilizer is not
  • Alter entry of gyroscopic stabilizer so it has mod_targets and mod_exclusions for laser_rifle, and extend mod_target_category to include ENERGY_WEAPONS
    • Check that error message screams at you for doing this twice (once for mod_targets and mod_exclusions and once for mod_target_category and mod_exclusion_category)
    • Check that you can install the gyroscopic stabilizer now.
  • Check that if you supply invalid ids to mod_exclusion and mod_exclusion_category it screams at you.

Additional context

Not ideal to be using gyroscopic stabilizers as the proof of concept, but I can't think of anything that I can even remotely justify as an example right now. Maybe someone can look over the gunmods to see if anything nonsensical jumps out.

Checklist

  • Document the changes in the appropriate location in the doc/ folder.
  • If documentation for this feature does not exist, please write it or at least note its lack in PR description.
  • New localizable fields need to be added to the lang/bn_extract_json_strings.sh script if it does not support them yet. (?)
  • If applicable, add checks on game load that would validate the loaded data.

Allows gunmods to exclude specific weapons/weapon categories.
@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 Feb 25, 2024
Copy link
Contributor

autofix-ci bot commented Feb 25, 2024

The Autofix app has found code style violation and automatically formatted this Pull Request.

I locally edit my commits (e.g: git, github desktop)

Please choose following options:

I'd like to accept the automated commit
  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.

This PR is complete and I don't want to edit it anymore

It's safe to ignore this message.

I edit this PR through web UI

You can ignore this message and continue working.

I have no idea what this message is talking about

You can ignore this message and continue working. If you find any problem, please ask for help and ping @scarf005.

@KheirFerrum KheirFerrum changed the title feat(gunmods): Add exclusions and exclusion categories to gunmods feat: Add exclusions and exclusion categories to gunmods Feb 25, 2024
@scarf005 scarf005 self-requested a review February 29, 2024 15:17
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

additional screenshots

image
image
image

src/item.cpp Outdated
} else if( !g_mod.usable.empty() || !g_mod.usable_category.empty() || !g_mod.exclusion.empty() ||
!g_mod.exclusion_category.empty() ) {
// First check that it's not explicitly excluded by id.
bool unusable = g_mod.exclusion.count( this->typeId() );
Copy link
Member

Choose a reason for hiding this comment

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

might make sense to rename it to excluded.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, how so?

Copy link
Member

@scarf005 scarf005 Mar 1, 2024

Choose a reason for hiding this comment

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

because this line is highly confusing?

            if( usable || unusable ) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I thought you were talking about "exclusion".

Per request for readability.
@KheirFerrum KheirFerrum requested a review from scarf005 March 1, 2024 18:43
@scarf005 scarf005 self-assigned this Mar 2, 2024
@scarf005 scarf005 merged commit 85f265a into cataclysmbnteam:main Mar 2, 2024
13 checks passed
@KheirFerrum KheirFerrum deleted the mod_exclusion_category branch March 2, 2024 04:09
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