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

sc2: Many typing and style fixes; fixed some broken logic functions #370

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

MatthewMarinets
Copy link

What is this fixing or adding?

Mypy fixes, and fixing some broken (always true) logic functions in locations.py highlighted by mypy.

This gets the typing issue count down by several hundred. The biggest improvement came from changing get_option_value()'s return type from Union[int, FrozenSet] to int (which isn't always accurate, but more accurate than the old one). Added a comment to it as well to discourage using it, which will hopefully get people to the syntax that actually gives the correct type.

Next biggest improvement was to rules.py, accurately annotating the functions/methods that are null-safe on their world parameter, and setting the player default value to -1 instead of None so the logic functions don't all scream.

I stayed away from pool_filter.py and those parts in init.py that I already fixed up in #369. I also tried to stay away from the mission_order subdirectory as I don't fully understand that code, I assume Salz is still touching it up occasionally, and it generally had a dozen issues per file. Outside of those places, the issue count in the sc2 world should be in the single-digits.

How was this tested?

Ran all unit tests. Ran mypy many times.

If this makes graphical changes, please attach screenshots.

image
(800+ of these are in alttp/core; used to be 1400~1500)

@Ziktofel Ziktofel merged commit 70f81eb into Ziktofel:sc2-next Dec 11, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants