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

Materials Roll Individually For Direct Attacks #55405

Merged
merged 3 commits into from
Feb 17, 2022

Conversation

bombasticSlacks
Copy link
Contributor

@bombasticSlacks bombasticSlacks commented Feb 15, 2022

Summary

None

Purpose of change

Materials on armor didn't work exactly how people thought but wanted to use it in PR's like #55316, where the desired outcome is materials may overlap, protect with only 1, or protect with none. Before the material roll was fixed for all materials so if you had two 80% materials you always got both or none, never that middle ground chance.

This fixes that.

Describe the solution

Made it so passing a negative value for roll to the relevant call stack makes it roll a chance per material at the lowest level.

So for UI and such you can still pass a fixed material roll (including 0 for legacy calcs).

Added a unit test for this stuff.

Describe alternatives you've considered

Testing

Added a unit test specifically testing for this. Using a large number of materials to minimize variance.

Additional context

This should effect basically no balance since almost no items are made of 2+ non 100% proportional coverage materials.

@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 Feb 15, 2022
@PlutusPleion
Copy link
Contributor

PlutusPleion commented Feb 15, 2022

Interesting, I didn't know it only used a single roll to check all materials. I thought it rolled on each material entry.

@bombasticSlacks
Copy link
Contributor Author

Interesting, I didn't know it only used a single roll to check all materials. I thought it rolled on each material entry.

your PR is why I checked it to make sure 😆

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Feb 15, 2022
@bombasticSlacks bombasticSlacks marked this pull request as ready for review February 15, 2022 18:31
@Maleclypse Maleclypse added [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. <Bugfix> This is a fix for a bug (or closes open issue) labels Feb 16, 2022
@kevingranade kevingranade merged commit 4741d2e into CleverRaven:master Feb 17, 2022
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 <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. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants