search

ben-wallis / Filtration

The Path of Exile loot filter editor
GNU General Public License v2.0
261 stars 61 forks source link

Bug of recognition attributes #17

Open smad2005 opened 9 years ago

smad2005 commented 9 years ago

Full support for all item filter attributes

Artificial example:

Show
 Rarity = "Normal" Rare
 SetTextColor 255 255 0
 SetBackgroundColor 255 255 255
 Height   1  3
 Width    3  1
 Sockets 2 4 6
 ItemLevel > 10 45
 DropLevel > 5 6
Quality 0 10 20
LinkedSockets  0 1 3
Sockets 0 1 3

pic

The operator != doesn't exist in game

ben-wallis commented 8 years ago

Fair point on != not existing in game, I'll remove it. What sort of syntax is this though?

Height   1  3
Width    3  1
Sockets 2 4 6
ItemLevel > 10 45
DropLevel > 5 6
Quality 0 10 20
LinkedSockets  0 1 3
Sockets 0 1 3

Do you have any links or sources that any of this is supported syntax?

smad2005 commented 8 years ago

Syntax has been described on wiki.

Every condition can have multiple values separated by a space.

Wiki

ben-wallis commented 8 years ago

Ok so assuming Sockets 2 4 6 works (haven't tested it yet) and only shows you items with 2, 4 or 6 sockets, what would ItemLevel > 10 45 do? It seems separating conditions with a space doesn't make sense unless an operator is omitted. The use cases for such syntax for non-text based block items seem unclear to me too - why would you ever want a filter that only shows you items with 0, 1 or 3 sockets? Or items with only 0 10 or 20 quality? I'm not saying I'm not going to implement it, just that I don't see where it'd ever be used.

smad2005 commented 8 years ago

I don't see where it'd ever be used.

You absolutely right, but you wrote that you have full syntax support, that mean if someone decided to use such constructions he also expects that your parser won't break his filter.

Full syntax on antlr

ben-wallis commented 8 years ago

Right so I just tested this in game, and found the following.

Show
    Sockets  1 3
    SetBackgroundColor 0 255 0

Hide

This syntax is valid and does work in game, so that's a legitimate bug in Filtration's script parsing. However, due to the object model in Filtration, I'm going to have to compromise here and create the above as two separate blocks, one for Sockets = 1, and one for Sockets = 3. This is due to the fact that all number based filter items inherit from NumberFilterPredicateBlockItem and have a NumberFilterPredicate property that pervades the entire code-base and is used extensively in the block item editor. Refactoring the way numeric block items are stored would be a major refactoring for very little benefit (since there are very few if any actual uses for this syntax). I think that not breaking existing scripts, but adding an extra block or two to maintain the functionality is acceptable.

Show
    Sockets > 1 3
    SetBackgroundColor 0 255 0

Hide

This syntax works in-game, but it ignores every number after the one immediately following the operator. Therefore, I'm proposing to fix Filtration's parsing to read up until the first number, and discard all numbers after as they don't work in-game and have no worth.

So, how's that sound?

ben-wallis commented 8 years ago

Oh, and your ANTLR parser looks pretty neat, is it in a state to be used as an external library? It'd be nice to integrate into Filtration as a check before beginning to parse the script, i.e. "Invalid Syntax on line x".

smad2005 commented 8 years ago

Example of use

https://github.com/smad2005/PoeHud/blob/9523ac4d2e8d9da34428ea2977cfb3c78b2a5cc9/src/Hud/Loot/ItemAlertPlugin.cs#L65-L71