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

Core: Add display name for item_links Option. #1952

Merged
merged 1 commit into from
Jul 22, 2023

Conversation

ThePhar
Copy link
Member

@ThePhar ThePhar commented Jul 6, 2023

What is this fixing or adding?

Just adds a display name. Mostly so spoiler prints Item Links instead of item_links.

Also re-sorts the imports in Option.py since my IDE does that. :P

How was this tested?

Looked at spoiler log after generating game.

If this makes graphical changes, please attach screenshots.

image
wow

@ThePhar ThePhar added is: documentation Improvements or additions to documentation. is: refactor/cleanup Improvements to code/output readability or organizization. affects: core Issues/PRs that touch core and may need additional validation. labels Jul 6, 2023
Comment on lines +9 to 13
from copy import deepcopy

from schema import And, Optional, Or, Schema

from schema import Schema, And, Or, Optional
from Utils import get_fuzzy_results
Copy link
Member

Choose a reason for hiding this comment

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

This looks odd.

I think most common is either no blank line or a single blank line between all imports and all from ... import.
Imports are typically sorted alphabetically.

Copy link
Member Author

Choose a reason for hiding this comment

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

The way my IDE is setup, all builtin imports are grouped together, then all pip package imports, then all local project imports.

Ultimately, it's just personal preference and if need be, I can easily revert that tweak.

Copy link
Member

Choose a reason for hiding this comment

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

PEP8 only requires you to sort each individual group. Groups are separated by a blank line. The 3-way split looks verbose to me.

It's generally OK here, but I'm not sure if reformatting all files you touch (assuming that's automatic) is a good idea.

@ThePhar ThePhar merged commit ecb1e0b into ArchipelagoMW:main Jul 22, 2023
@ThePhar ThePhar deleted the item_links-display_name branch July 22, 2023 00:31
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
kl3cks7r pushed a commit to kl3cks7r/Archipelago that referenced this pull request Dec 15, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: documentation Improvements or additions to documentation. is: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants