-
Notifications
You must be signed in to change notification settings - Fork 43
Importer and Edito UI Changes #779
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
base: main
Are you sure you want to change the base?
Changes from all commits
231954b
286fb76
aaa5bbe
55ca3a7
d898b17
dd7c587
d7ebece
b8c08ab
81d9a32
4fd7835
4945c68
4b41332
5bec0bb
c5c9545
2700686
8f66b7e
49a6f6c
2598fc0
7dc6fc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ def parse_data(cls, data: str | list[str] | list[str | float] | dict[str, str | | |
| class AffixFilterModel(AffixAspectFilterModel): | ||
| model_config = ConfigDict(extra="forbid", populate_by_name=True) | ||
| want_greater: bool = False | ||
| required: bool = False | ||
| min_percent_of_affix: int = Field(default=0, alias="minPercentOfAffix") | ||
|
|
||
| @field_validator("name") | ||
|
|
@@ -94,12 +95,13 @@ def model_validator(self) -> AffixFilterCountModel: | |
| self.max_count = len(self.count) | ||
| self.model_fields_set.remove("min_count") | ||
| self.model_fields_set.remove("max_count") | ||
|
|
||
| req_count = sum(1 for a in self.count if a.required) | ||
| self.min_count = max(self.min_count, req_count) | ||
|
|
||
| if self.min_count > self.max_count: | ||
| msg = "minCount must be smaller than maxCount" | ||
| raise ValueError(msg) | ||
| if not self.count: | ||
| msg = "count must not be empty" | ||
| raise ValueError(msg) | ||
| return self | ||
|
|
||
|
|
||
|
|
@@ -140,6 +142,10 @@ class GlobalUniqueModel(BaseModel): | |
| min_greater_affix_count: int = Field(default=0, alias="minGreaterAffixCount") | ||
| min_percent_of_aspect: int = Field(default=0, alias="minPercentOfAspect") | ||
| min_power: int = Field(default=0, alias="minPower") | ||
| item_type: list[ItemType] = Field(default=[], alias="itemType") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These make no sense here and were already removed previously and intentionally
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the code uses this field to immediately skip items that don't match the intended item category.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would a user want to say "Keep all unique helmets". It literally won't happen, which is why it was removed previously. And the other functionality is similarly unnecessary or was added to the Affixes. |
||
| affix_pool: list[AffixFilterCountModel] = Field(default=[], alias="affixPool") | ||
| inherent_pool: list[AffixFilterCountModel] = Field(default=[], alias="inherentPool") | ||
| unique_aspect: list[AspectUniqueFilterModel] = Field(default=[], alias="uniqueAspect") | ||
|
|
||
| @field_validator("min_power") | ||
| @classmethod | ||
|
|
@@ -159,6 +165,29 @@ def count_validator(cls, v: int) -> int: | |
| def percent_validator(cls, v: int) -> int: | ||
| return validate_percent(v) | ||
|
|
||
| @field_validator("item_type", mode="before") | ||
| @classmethod | ||
| def parse_item_type(cls, data: str | list[str]) -> list[str]: | ||
| return _parse_item_type_or_rarities(data) | ||
|
|
||
| @field_validator("unique_aspect", mode="before") | ||
| @classmethod | ||
| def parse_unique_aspect(cls, data: dict | list[dict] | None) -> list[dict]: | ||
| if not data: | ||
| return [] | ||
| if isinstance(data, dict): | ||
| return [data] | ||
| return data | ||
|
|
||
| @model_validator(mode="after") | ||
| def unique_aspect_names_must_be_unique(self) -> GlobalUniqueModel: | ||
| if len({aspect.name for aspect in self.unique_aspect}) != len(self.unique_aspect): | ||
| msg = "uniqueAspect names must be unique" | ||
| raise ValueError(msg) | ||
| if not self.affix_pool: | ||
| self.affix_pool = [AffixFilterCountModel(count=[], min_count=0)] | ||
| return self | ||
|
|
||
|
|
||
| class ItemFilterModel(BaseModel): | ||
| model_config = ConfigDict(extra="forbid", populate_by_name=True) | ||
|
|
@@ -201,6 +230,8 @@ def unique_aspect_names_must_be_unique(self) -> ItemFilterModel: | |
| if len({aspect.name for aspect in self.unique_aspect}) != len(self.unique_aspect): | ||
| msg = "uniqueAspect names must be unique" | ||
| raise ValueError(msg) | ||
| if not self.affix_pool: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. affix_pool can never be none, what is this trying to accomplish?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is a safeguard that ensures the filtering engine has a structured object to work with, even when the user provides minimal configuration. It turns "no affix requirements" into an explicit "zero-requirement match" so the program logic remains consistent.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry I don't know what that's saying. affix_pool has a default value of []. There is no situation where it will need to be reinitialized. If there is a situation we probably need to fix a problem elsewhere |
||
| self.affix_pool = [AffixFilterCountModel(count=[], min_count=0)] | ||
| return self | ||
|
|
||
|
|
||
|
|
@@ -321,11 +352,13 @@ class ProfileModel(BaseModel): | |
| aspect_upgrades: list[str] = Field(default=[], alias="AspectUpgrades") | ||
| global_uniques: list[GlobalUniqueModel] = Field(default=[], alias="GlobalUniques") | ||
| name: str | ||
| class_name: str = Field(default="unknown", alias="ClassName") | ||
| sigils: SigilFilterModel = Field( | ||
| default=SigilFilterModel(blacklist=[], whitelist=[], priority=SigilPriority.blacklist), alias="Sigils" | ||
| ) | ||
| tributes: list[TributeFilterModel] = Field(default=[], alias="Tributes") | ||
| paragon: dict[str, object] | list[dict[str, object]] | None = Field(default=None, alias="Paragon") | ||
| source_url: str = Field(default="", alias="SourceUrl") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you make sure anywhere the paragon board used a source_url is either removed or it uses this one? I think it only stored it for no particular reason and can probably just get removed |
||
|
|
||
| @model_validator(mode="before") | ||
| def aspects_must_exist(self) -> ProfileModel: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,6 @@ | |
| ) | ||
| from src.dataloader import Dataloader | ||
| from src.gui.importer.gui_common import ( | ||
| add_mythics_to_filters, | ||
| add_to_profiles, | ||
| build_default_profile_file_name, | ||
| fix_offhand_type, | ||
|
|
@@ -95,7 +94,6 @@ def import_d4builds(config: ImportConfig, driver: ChromiumDriver = None): | |
| raise D4BuildsError(msg) | ||
| slot_to_unique_name_map = _get_item_slots(data=data) | ||
| finished_filters = [] | ||
| mythic_names = [] | ||
| aspect_upgrade_filters = _get_legendary_aspects(data=data) | ||
| for item in items[0]: | ||
| item_filter = ItemFilterModel() | ||
|
|
@@ -115,9 +113,6 @@ def import_d4builds(config: ImportConfig, driver: ChromiumDriver = None): | |
|
|
||
| if slot_to_unique_name_map[slot]: | ||
| unique_name, rarity = slot_to_unique_name_map[slot] | ||
| if rarity == ItemRarity.Mythic: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't import affixes for mythics. This change alone has us import affixes. You can look at how the code was before I implemented multiple aspects if you want to see how to handle this
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And note this applies to all of the importers |
||
| mythic_names.append(unique_name) | ||
| continue | ||
| try: | ||
| item_filter.unique_aspect = [AspectUniqueFilterModel(name=unique_name)] | ||
| except Exception: | ||
|
|
@@ -198,9 +193,9 @@ def import_d4builds(config: ImportConfig, driver: ChromiumDriver = None): | |
| filter_name = f"{filter_name_template}{i}" | ||
| i += 1 | ||
| finished_filters.append({filter_name: item_filter}) | ||
| # Place all mythics in a single filter | ||
| add_mythics_to_filters(mythic_names, finished_filters) | ||
| profile = ProfileModel(name="imported profile", Affixes=sort_profile_filters(finished_filters)) | ||
| profile = ProfileModel( | ||
| name="imported profile", affixes=sort_profile_filters(finished_filters), class_name=class_name, source_url=url | ||
| ) | ||
| if config.import_aspect_upgrades and aspect_upgrade_filters: | ||
| profile.aspect_upgrades = aspect_upgrade_filters | ||
|
|
||
|
|
@@ -210,6 +205,7 @@ def import_d4builds(config: ImportConfig, driver: ChromiumDriver = None): | |
| season_number=season_number, | ||
| build_header=build_header, | ||
| variant_name=variant_name, | ||
| filename_components=config.filename_components, | ||
| ) | ||
|
|
||
| # Optionally embed Paragon data into the profile model before saving | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what this required flag is but it doesn't seem relevant to a profile editor and importer overhaul. I'm unclear how this works differently than adding another count block, which you also support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The required option would let you set a specific affix to required which would be along with any count settings you might have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The count block already accomplishes this, it's not good to add new functionality here where it can be avoided