3.16 Export Fixes for Items and Passives #38

angelic-knight · 2021-11-04T03:53:04Z

Abstract

Allow exporting of passives and items for 3.16.
Additionally, include some generic fixes that I developed to aid in 3.16 updates.

Action Taken

Added handling to disambiguate the Energy Blade skill gem item from the sword items that it creates. This fixes [Items export - 3.16] Energy Blade fails to export due to error #35
Added support for the armour defence ranges. This fixes [Item export - 3.16] Add new stat ranges for armors #29 on the PyPoE side.
Added the 'Armour' type to the item class map.
Stopped using the crashing hideout doodad conflict resolver. This is related to [Items export - 3.16] Hideout Doodads Conflict Resolver Relies on Non-existent column #34.
Updated the MAP_FRAGMENT_FAMILIES enum with new values and remapped existing values so they're accurate.
Fixed the parsing of PassiveSkillGraph.psg so that the passive export will work.
Made various fixes to dat specs to allow exporting to work. I reverted some field
renaming that was previously added and merged in from /3.16-update, as it was breaking other places that were expecting those field names. This fixes [Items export - 3.16] Projectiles.dat parsing is broken #33.

Additionally, implemented generic fixes as described in #37, but copied here:

In util.py, revert my previous change which sanitized double quotes. Another commit by someone else handled this better upstream of the change I'm reverting.
In item.py, print out the rowid of the item being processed every so often (between 100 and 200 times total, in equal length increments)
In lua.py, fix the crafting item class categories keys parameter to match what we use in our wiki. This fixes issue 24
In passives.py, prevent crashing out when a passive has no icon path, and give a warning instead.
In skill.py, print out some rowids and add in some typo fixes which were I think were in /patches already.

Caveats

I did not add a new hideout doodad conflict resolver, so [Items export - 3.16] Hideout Doodads Conflict Resolver Relies on Non-existent column #34 is still alive and causing problems. Of note is that it's blocking the totems from being exported.
The wiki side will need to be updated for the armour defence ranges still.
We should probably update names of fields in stable.py at some point to match the updates to the dat spec repository (they had been updated but I'm reverting them here as it was simpler than changing other places in PyPoE).
In this PR, and in all my others up to this point, I have not changed or tested image exports.

FAO

@pm5k @acbeaumo @Journeytojah

…don't have proper translations from internal descriptions to normal text.

…m's table name.

…fixed this elsewhere.

Adding printing to indicate current rows being processed (up to 200 row IDs and names will print) during exporting. Re-evaluated and updated the MAP_FRAGMENTS_FAMILIES enum Fixed some specs that were erroring out during item exporting.

…ssues where conflicts aren't resolved) Fix the HeistJobs.dat spec to include the new columns for 3.16

Supporting exporting passives that don't have icon paths. Revert some spec column name changes that were causing issues. I need to run the spec autogeneration sometime and then update PyPoE hardcoded references to use the new values instead of manually editing the spec.

Adding disambiguation suffixes for Energy Blade skill gem and Energy Blade swords (that you get from using the skill gem) Removed old fix to an issue since it was fixed a better way (in fields.py). Added extra printing to skill export.

pm5k · 2021-11-06T09:11:47Z

PyPoE/cli/exporter/wiki/parsers/item.py

+        # ('HideoutNPCsKey', {
+        #     'template': 'master',
+        #     'format': lambda v: v['Hideout_NPCsKey']['Name'],
+        #     'condition': lambda v: v is not None,
+        # }),
+        # ('MasterLevel', {
+        #     'template': 'master_level_requirement',
+        # }),
+        # ('FavourCost', {
+        #     'template': 'master_favour_cost',
+        # }),


Is this a case of YAGNI?

Will we ever have any need for this? @Journeytojah @angelic-knight
If not needed, just remove outright instead of comment. But please check with one another first.

I think this should be safe to delete. @Journeytojah, what's your take on it?

This is fine to remove.

I've removed this and surrounding related code.

pm5k · 2021-11-06T09:13:25Z

PyPoE/cli/exporter/wiki/parsers/item.py

+    '''
+    This defines the expected data elements for an item class.
+    '''


It seems to me like we're inside of a method here, which would mean that we should use line comments here instead of docstrings. Can we please change this to be # This defines the expected data elements for an item class. insead?

It's within a class, not a method.
I'm tweaking it so intellisense will pick up on the docstring.

pm5k · 2021-11-06T09:14:29Z

PyPoE/cli/exporter/wiki/parsers/item.py

@@ -3281,6 +3321,23 @@ def _export(self, parsed_args, items):

        return r

+    def _print_item_rowid(self, parsed_args, base_item_type):
+        #Don't print anything if not running in the rowid mode.
+        if (not 'start' in vars(parsed_args).keys()) or (not 'end' in vars(parsed_args).keys()):


Same comment as #37 (comment)

pm5k · 2021-11-06T09:16:30Z

PyPoE/cli/exporter/wiki/parsers/lua.py

-                    'Id']['ItemDisplayString' + row['RarityKey'].name_upper][
-                    'Text']
+            if row['BeastRarity'] != RARITY.ANY:
+                components[-1]['rarity'] = self.rr['ClientStrings.dat'].index['Id']['ItemDisplayString' + row['BeastRarity'].name_upper]['Text']


This one's a doozy to follow by-eye. Can we try:

if row['BeastRarity'] != RARITY.ANY: display_string = 'ItemDisplayString' + row['BeastRarity'].name_upper components[-1]['rarity'] = self.rr['ClientStrings.dat'].index['Id'][display_string]['Text']

pm5k · 2021-11-06T09:16:59Z

PyPoE/cli/exporter/wiki/parsers/passives.py

+            #atlas_start_node doesn't have an icon path
+            else:
+                data['icon'] = ''
+                warnings.warn('Icon path file not found for {}: {}'.format(passive['Id'], passive['Name']))


Same comment as #37 (comment)

pm5k · 2021-11-06T09:17:10Z

PyPoE/cli/exporter/wiki/parsers/skill.py

@@ -742,7 +742,7 @@ def export(self, parsed_args, skills):
            if not parsed_args.allow_skill_gems and skill in \
                    self.rr['SkillGems.dat'].index['GrantedEffectsKey']:
                console(
-                    'Skipping skill gem skill "%s"' % skill['Id'],
+                    'Skipping skill gem skill "{}" at row {}'.format(skill['Id'], skill.rowid),


#37 (comment)

pm5k · 2021-11-06T09:17:15Z

PyPoE/cli/exporter/wiki/parsers/skill.py

@@ -751,15 +751,14 @@ def export(self, parsed_args, skills):
                self._skill(ge=skill, infobox=data, parsed_args=parsed_args)
            except Exception as e:
                console(
-                    'Error when parsing skill "%s":' % skill['Id'],
+                    'Error when parsing skill "{}" at {}:'.format(skill['Id'], skill.rowid),


#37 (comment)

pm5k · 2021-11-06T09:17:35Z

PyPoE/poe/file/psg.py

        unknown = struct.unpack_from(
            '<' + 'B'*unknown_length, data, offset=offset
        )
        offset += 1*unknown_length

        root_length = struct.unpack_from('<I', data, offset=offset)[0]
+        if(root_length > 1000):
+            raise ValueError(
+                'root_length is unrealistically large at {}.\nStopping to prevent allocating too much memory'.format(root_length)


#37 (comment)

pm5k · 2021-11-06T09:18:10Z

PyPoE/poe/file/translations.py

+                        warnings.warn('Broken quantifier "%s" - Error: %s' %
+                                      (string, e.args[0]), TranslationWarning)


#37 (comment)

pm5k · 2021-11-06T09:18:30Z

PyPoE/poe/file/translations.py

+                warnings.warn('Duplicate id "%s"' %
+                              translation_id, DuplicateIdentifierWarning)


#37 (comment)

pm5k · 2021-11-06T09:22:00Z

I am also concerned that you have seemingly doubled-up your own workload since a lot of changes from #37 show up here. The reason we keep dev separate from patches is so that some fixes don't affect certain mappings/schema.

I wonder if there's specific requirements for your branch to use stuff you already submitted into the dev PR (#37), is that the case here?

@Journeytojah would be good to get your input here.

angelic-knight · 2021-11-06T19:25:11Z

I've removed all the changes from #37 except for the handling of passives that don't have icon paths, as that was crashing in 3.16.

…iated with doodads.

angelic-knight added 11 commits October 27, 2021 01:07

First few fixes for exporting mods. This is mostly .dat spec fixes.

b2ef4c2

Enough spec fixes to export all 26k mods without crashing. Many mods …

b34b707

…don't have proper translations from internal descriptions to normal text.

Fixing Project-Path-of-Exile-Wiki#24, where our PyPoE was using Fando…

557a214

…m's table name.

Merge branch 'dev' into 3.16-updates

6397e03

Merge remote-tracking branch 'upstream/3.16-update' into 3.16-update

30420c1

Remove redundant quote sanitizing. Merged commit 5c4db74 should have …

f702e9a

…fixed this elsewhere.

Stop using broken hideout doodad conflict resolver (this introduces i…

860ef96

…ssues where conflicts aren't resolved) Fix the HeistJobs.dat spec to include the new columns for 3.16

Energy Blade & Cleanup

dcd5e3c

Adding disambiguation suffixes for Energy Blade skill gem and Energy Blade swords (that you get from using the skill gem) Removed old fix to an issue since it was fixed a better way (in fields.py). Added extra printing to skill export.

Fix printing item rowids so it won't crash when not in rowid mode.

4fcc59f

angelic-knight mentioned this pull request Nov 5, 2021

Fix the Scourge Map Series export #39

Merged

pm5k requested changes Nov 6, 2021

View reviewed changes

angelic-knight added 3 commits November 6, 2021 12:33

Fixing PR issues from PR Project-Path-of-Exile-Wiki#37

59bcc8a

Removing unnecessary general fixes from /patches branch

acd3c82

Fixing code style issues from PR review.

6176bdd

Removing outdated hideout doodad code that assumed masters were assoc…

640a1fc

…iated with doodads.

pm5k approved these changes Nov 9, 2021

View reviewed changes

Merge branch 'patches' into 3.16-update

c38252e

pm5k merged commit c1682a8 into Project-Path-of-Exile-Wiki:patches Nov 9, 2021

pm5k mentioned this pull request Nov 9, 2021

3.16 - Pre-release merge, patch version 2. #48

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3.16 Export Fixes for Items and Passives #38

3.16 Export Fixes for Items and Passives #38

angelic-knight commented Nov 4, 2021 •

edited

Loading

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

Journeytojah Nov 7, 2021

angelic-knight Nov 7, 2021

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

pm5k Nov 6, 2021

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

pm5k Nov 6, 2021

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

pm5k Nov 6, 2021

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

pm5k Nov 6, 2021

angelic-knight Nov 6, 2021

pm5k commented Nov 6, 2021

angelic-knight commented Nov 6, 2021

		warnings.warn('Broken quantifier "%s" - Error: %s' %
		(string, e.args[0]), TranslationWarning)

		warnings.warn('Duplicate id "%s"' %
		translation_id, DuplicateIdentifierWarning)

3.16 Export Fixes for Items and Passives #38

3.16 Export Fixes for Items and Passives #38

Conversation

angelic-knight commented Nov 4, 2021 • edited Loading

Abstract

Action Taken

Caveats

FAO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pm5k commented Nov 6, 2021

angelic-knight commented Nov 6, 2021

angelic-knight commented Nov 4, 2021 •

edited

Loading