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 exception when price checking corrupted gems #403

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

Ati1707
Copy link
Contributor

@Ati1707 Ati1707 commented Dec 26, 2024

This part of the parser does not work or I don't seem to grasp the functionality of this.

var canBeVaalGem = itemRarity == Rarity.Gem && parsingItem.Blocks.Count > 7;
if (canBeVaalGem && data.NameAndTypeDictionary.TryGetValue(parsingItem.Blocks[5].Lines[0].Text, out var vaalGem))
{
return vaalGem.First();
}

It tries to parse an empty line which creates the error since there is no text.

The parser already gets the corrupted state from the property parse I didnt go through the code but I assume that property is used to set the variable for the price check page as well.

data.NameAndTypeDictionary.TryGetValue(parsingItem.Blocks[5].Lines[0].Text
Even if that empty block would not be there it would still parse the wrong text.
Thats why I removed the vaal gem part from the code.

These are the blocks from zero to last element as you can see [] would be the sixth element even if that element would not be there it would parse Skills can be managed in the Skills Panel.

The gem text was taken from the mentioned issue:

[Item Class: Skill Gems, Rarity: Gem, Explosive Shot]
[Attack, AoE, Ammunition, Projectile, Fire, Detonator, Level: 16 (augmented), 15 Levels from Gem...]
[Requirements:, Level: 64, Crossbow, Str: 80, Dex: 80]
[Sockets: G G G ]
[...]
[]
[Skills can be managed in the Skills Panel.]
[Corrupted]

Closes #384

@domialex
Copy link
Contributor

@leMicin This was for poe1 vaal gems?

@leMicin
Copy link
Contributor

leMicin commented Dec 26, 2024

Yes this was a PoE1 condition.

@Ati1707 I believe the unit tests caught it failing. You should be able to exclude this vaal check by wrapping the whole block with something like item.Metadata.Game == PathOfExile condition.

@Ati1707
Copy link
Contributor Author

Ati1707 commented Dec 26, 2024

Yes this was a PoE1 condition.

@Ati1707 I believe the unit tests caught it failing. You should be able to exclude this vaal check by wrapping the whole block with something like item.Metadata.Game == PathOfExile condition.

Thanks. I was testing poe1 gems yesterday as well but it was late and apparently I made a mistake because I got an error yesterday.
In that scope it doesn't have access to the metadata so I decided to instead check if lines in the sixth block exists because they dont exist in poe2 but do in poe1.
I tried a corrupted,non corrupted poe2 gem and corrupted, vaal corrupted poe1 gem. They all went through without any issues and the tests passed as well locally.

@leMicin leMicin merged commit 73c6e62 into Sidekick-Poe:main Dec 27, 2024
@Ati1707 Ati1707 deleted the fix-corrupt-crash branch December 27, 2024 11:54
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.

Corrupted Explosive shot error/hangup
3 participants