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 ammo proportional/relative member assignment #47567

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

Qrox
Copy link
Contributor

@Qrox Qrox commented Feb 17, 2021

Summary

Bugfixes "Fix ammo proportional/relative member assignment"

Purpose of change

Fixes #47565.

Describe the solution

Revert to using assign in islot_ammo::load.

Testing

Checked in game and ammo properties were correctly assigned according to proportional/relative values.

Additional context

We might want to check if a proportional or relative field is unused, but a fix is not straighfoward because these fields are accessed in different places and it might not be easy to check which are unvisited.

@anothersimulacrum
Copy link
Member

#43144 will fix this (though after 0.F)

@KorGgenT
Copy link
Member

yeah i'd much rather we fix this correctly instead of backpedaling. i'd like us to move toward using mandatory and optional rather than assign, because the default values are then much more visible. i know that assign also has the functionality where there's an acceptable range of values which optional and mandatory doesn't have, but the part of manda

@ZhilkinSerg
Copy link
Contributor

Closing blocker now. Will be fixed properly later.

@ZhilkinSerg ZhilkinSerg merged commit a4df73f into CleverRaven:master Feb 18, 2021
@ZhilkinSerg ZhilkinSerg added the <Bugfix> This is a fix for a bug (or closes open issue) label Feb 18, 2021
@Qrox Qrox deleted the ammo branch February 20, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON relative/proportional flags for ammo don't work
4 participants