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

Fix telescope eyes not preventing eye mutations #37849

Merged
merged 4 commits into from
Feb 9, 2020

Conversation

Ramza13
Copy link
Contributor

@Ramza13 Ramza13 commented Feb 9, 2020

Summary

SUMMARY: Bugfixes "Fix telescope eyes allowing eye mutations to be gained"

Purpose of change

Fixes #21407
Fixes #15211
Prevents gaining eye mutations with telescoping eyes installed

Describe the solution

Added a check to is_mutation_ok to prevent gaining eye mutations while having telescope eyes installed. Also expanded the number of eye mutations removed to correspond with the prevented ones.

Describe alternatives you've considered

Testing

Additional context

@Night-Pryanik
Copy link
Contributor

My only concern is hardcoded list of affected bionics. Maybe replace it with some new flag on bionics?

@Fris0uman
Copy link
Contributor

Yes please don't hardcode it, there is already a thing for CBM to remove mutations if you could expend it to also forbid getting those mutations that would be great.

@Ramza13
Copy link
Contributor Author

Ramza13 commented Feb 9, 2020

Thanks for the feedback! I have changed it to now work for anything using the canceled mutations tag and added the correct tags to bionics.json.

@Amoebka
Copy link
Contributor

Amoebka commented Feb 9, 2020

This behaviour isn't really correct in the general case either. Just because a CBM removes certain mutations on installation doesn't mean it should also always prevent them from reappearing. A CBM that replaces your eyes obviously removes eye mutations, but it doesn't prevent you from growing another pair of eyes elsewhere after the fact.
Ultimately, this is something that should be set case by case, so a new flag is needed.

@Ramza13
Copy link
Contributor Author

Ramza13 commented Feb 9, 2020

At the moment this is the only time this tag is being used so I guess I could rename it to be more accurate but I'm not sure if that's a good idea.

@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Bionics CBM (Compact Bionic Modules) Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies <Bugfix> This is a fix for a bug (or closes open issue) labels Feb 9, 2020
@ZhilkinSerg ZhilkinSerg merged commit 7a38932 into CleverRaven:master Feb 9, 2020
@Ramza13 Ramza13 deleted the telescope_eyes branch February 9, 2020 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bionics CBM (Compact Bionic Modules) <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies
Projects
None yet
5 participants