Syntax Ch-ch-changes ?

Ludovicus_Maior's picture

So I have had this double-secret set of changes that I had tried to make to merge the parsers from the various WoW-Pro addons and I have decided that the changes are ugly and deserve to be flushed.  Part of the problem is that the current guide syntax is contextual and not really regular.

Take the following step line from a guide:

C Curing The Incurable|QID|13040|N|Loot Venom Sacs off the Nerubians.|S|

Note that—although pipe ('|') characters separate different sections—some tags take arguments and some don't, and that the first part is handled differently from the rest. In effect, you have to know what the tags are order to interpret the line.  That logic is buried in a mess of code and regular expression matching and lots of nested conditionals.

Now let us suppose the step line looked like this:

STEP=C|LABEL=Curing The Incurable|QID=13040|N=Loot Venom Sacs off the Nerubians.|S|

Now the syntax is regular. Tags are separated from each other by a pipe, and an argument is separated from its tag by an equal sign. Internally we could represent each line by an LUA table like the following:

{STEP="C", LABEL="Curing The Incurable", QID="13040", N="Loot Venom Sacs off the Nerubians.", S=true}

Then the parser would be the same for everything and would be pretty simple.

So what about the mechanics of each step?  Well, the tags basically fall into three categories.  There are tags that control whether a step is presented at all (let us call these filter tags) and others that provide information (info tags?) and others that control completion (completion tags?).  Filter tags do not depend on the step type, most info tags are independent on the step type, but most completion tags depend on the step type.

So now things can get fun.   The interpretation of a step line could be divided into four phases:

  1. We pass the line through a filter table/functions which check to see that every filter tag says pass;
  2. We then do a step-dependent check to see if the conditions for completion have already been fulfilled;
  3. If not, we present the step, using the info tags;
  4. And then we do a step-dependent check to see if the conditions for completion have  been fulfilled.

Once the process for interpreting steps is regularized, and the differences encapsulated in functions and data, each addon could then inherit the default rules from the base addon and only implement changes to the base.  But probably 90% of the logic would be shared.

Musings for a Sunday evening, when I am too tired to play, after running my first five-mile race for this year.  Uggh, gotta get into better shape!

PS: Got Loremaster for Northrend, now I need to finish ALL the dungeons so I can say I have covered the LK material and moving on to Cata.

Comments

Ludovicus_Maior's picture

So how many syntax errors are there in the guides?

There is a phrase in computer science that I hold dear to heart: GIGO.

If the data you are working on (guides in this case) are not clean before your start working with them (changing syntax), you may be unpleasantly surprised at the results.

So I wrote a little syntax checker for guide files.  It performs pretty weak checks on the tags, their arguments, checks that numbers are really numbers and things like that.   So I ran it on the current guide files and found:

421 errors

Now, some of these may not be errors and I may need to tweak the checker.   But a quick look at the start shows some pretty blantant errors:


# Checking ../WoWPro_Leveling/Alliance/01_05_Gylin_Dwarf_Starter.lua
# Checking ../WoWPro_Leveling/Alliance/01_05_Gylin_Gnome_Starter.lua
# Checking ../WoWPro_Leveling/Alliance/01_11_Kurich_Elwynn_Forest.lua
! Line 104 for step C has unknown tag [48.63,44.37]: [C Immolation|N|Learn Immolate then use it on the training dummies. |QID|26914|C|Warlock|R|Human|48.63,44.37|]
! Line 162 for step C has unknown tag [T Fear No Evil]: [C Fear No Evil|N|Revive 4 Injured Soldiers. | |QID|28812|C|Warlock|R|Human|M|48.40,35.79|T Fear No Evil|QID|28806|C|Hunter|R|Human|M|48.49,38.16|]
! Line 197 for step T has unknown tag [T Blackrock Invasion]: [T Extinguishing Hope|QID|26391|R|Human|M|48.15,42.52|T Blackrock Invasion|QID|26389|R|Human|M|48.20,42.06|]
# Checking ../WoWPro_Leveling/Alliance/01_12_Bitsem_Teldrassil.lua
! Line 31 for step C has unknown tag [Kill Young Nightsabers.]: [C The Balance of Nature|QID|28713|M|58.23,40.16|Kill Young Nightsabers.|]
! Line 107 for step A has unknown tag [28727]: [A Signs of Things to Come|QID|28728|28727|NC|N|From Tarindrella. She'll teleport you back to Dentaria Silverglade.|]
# Checking ../WoWPro_Leveling/Alliance/01_12_Snowflakes_Azuremyst.lua
! Line 46 for step f has unknown tag [10302]: [f Seat of the Naaru|QID|9625|10302|R|Human, Worgen, Gnome, Dwarf, Night Elf|Z|The Exodar|M|54.22,36.55|N|At Stephanos.|]
! Line 68 for step C Bad coord 49.97S: [C Botanical Legwork|QID|9799|M|74.39,49.97S|N|Collect Corrupted Flowers.|]
# Checking ../WoWPro_Leveling/Alliance/01_13_Rpotor_Worgen_Starter.lua
! Line 127 for step A has unknown tag [From Huntsman Blake.]: [A Safety in Numbers |QID|14290|M|71.3,61.4|C|Hunter||From Huntsman Blake.|]
! Line 146 for step A has unknown tag [From Captain Broderick.]: [A The Prison Rooftop |QID|28850|M|57.9,75.3|From Captain Broderick.|]
! Line 159 for step T has unknown tag [Go southwest/west, take a right around the house and you'll find a cellar door. Click it and it will open, revealing a staircase. Go down the stairs to Josiah Avery.]: [T The Rebel Lord's Arsenal |QID|14159|M|55.48,81.59;56.63,85.43|Go southwest/west, take a right around the house and you'll find a cellar door. Click it and it will open, revealing a staircase. Go down the stairs to Josiah Avery.|]
! Line 187 for step T has unknown tag [To Tobias.]: [T By Blood and Ash |QID|14218|M|40.3,39.5|To Tobias.|]
! Line 204 for step T has unknown tag [To Crate of Mandrake Essence.]: [T In Need of Ingredients|QID|14320|M|32.90,66.32|To Crate of Mandrake Essence.|]
! Line 239 for step C has 3 M coords: [C Kill Captain Morris|QID|14382|M|29,54,74.23; 27.2,80.0|QO|Captain Morris slain: 1/1|N|Go to another catapult, kill the Forsaken Machinist, launch yourself onto the other ship. Go downstairs and kill Captain Morris.|]
! Line 408 for step f has too short a title: [f |QID|26385|M|36.67,47.91|Z|Darnassus|N|At Leora.|]

Ludovicus_Maior's picture

Feedback appreciated, even through a hangover

Keeping things short and readable is important.   The current variation in syntax is:

STEP=C;Curing The Incurable|QID=13040|N=Loot Venom Sacs off the Nerubians.|S

Note that the terminal '|' is gone, as it is a separator and a terminal one would imply and empty tag at the end.  The only extra characters are the 'STEP=' at the beginning.  Now if I permit empty tags and treat them special, then I can get away with:

=C;Curing The Incurable|QID=13040|N=Loot Venom Sacs off the Nerubians.|S

For a net change of zero characters.   You split along the '|' characters, iterate over the split items, trim spaces, split on an '=' sign, trim spaces for both they tag and value and we are done.  Individual tags decide if ';' is a delimiter or not.   An empty tag is treated as the 'STEP' tag.  An empty value can be defaulted to true.

 

PS: This change can be implemented for the next release.  It means writing a program to transform the current guides, and then modifying the current parser to match, but still use the same data structures internally.  The shift to move the logic out of the parser and into a handler table can come later.  Gotta break it up into changes that are small and take no more than a day to implement so I can use them for a week to test. V2.2.0?

Then the more intrusive changes can happen bit by bit. 

Oy, gotta get showered and go to work!

 

Jiyambi's picture

I like everything but the

I like everything but the opening =C;, it's so icky looking. But I definitely understand the need for a uniform format, honestly if we're leaving the =C; in there, I'd say leave the STEP in there too. It seems actually more readable that way.

As for the PS paragraph, /agree with everything said. As always, let me know if there's anything I can do to help, though the next couple weeks will be even busier than normal for me!

Great work, thanks as always Smiling

What about [ ] to define all that is defined as a step

So you would have:

[C=Curing The Incurable|QID=13040;13041;13042|N=Loot Venom Sacs off the Nerubians.|S]

Ludovicus_Maior's picture

Step Types as Keys

I had considered:

[C=Curing The Incurable|QID=13040;13041;13042|N=Loot Venom Sacs off the Nerubians.|S]

but then you would have to test for:

[A=Curing The Incurable|QID=13040;13041;13042|N=Loot Venom Sacs off the Nerubians.|S][N=Curing The Incurable|QID=13040;13041;13042|N=Loot Venom Sacs off the Nerubians.|S][R=Curing The Incurable|QID=13040;13041;13042|N=Loot Venom Sacs off the Nerubians.|S]

for the different step types, probing each possiblle step type as a key in the table.  Seemed kinda nasty.  We have a LOT of different step types. It seemed easier to probe once and then indirect to a handler for the step type,.

But I have enjoyed too much red wine and need to sleep :-).

 

You could do a replace

To be consistant, you could do this displayed in your guide:

[A;Curing The Incurable|QID;13040;13041;13042|N;Loot Venom Sacs off the Nerubians.|S]
[N;Curing The Incurable|QID;13040;13041;13042|N;Loot Venom Sacs off the Nerubians.|S]
[N;Curing The Incurable|QID;13040;13041;13042|N;Loot Venom Sacs off the Nerubians.|S]

In the parser do a replace for [ with STEP:

string.gsub(string, "[", "STEP")

opt1,opt2,op3,etc = string.split("|",string)

Then do your processing of the options.

Ludovicus_Maior's picture

Doh! Wine does inhibit synapses!

Ah, I see. Just tack on the STEP= to the front of the line.  So why do you want the []'s s to bracket the step?  Is the rest of the line ignored and a comment?

Lets us [N;Curing The Incurable|QID;13040;13041;13042|N;Loot Venom Sacs off the Nerubians.|S] go to work?

versus:

N;Curing The Incurable|QID=13040;13041;13042|N=Loot Venom Sacs off the Nerubians.|S

with no permitted extra text.  The first field is special, the rest are order independent.

Not really....

Jiyambi commented that the =C was "so icky looking", and I was looking at alternatives.  The ] at the end is really just optional to signal the end of the step sequence as the parser read in the line.

Also, you may want to look at replacing the "=" with ";" consistantly through the parser.  That way you can break it down by action, option1, option2, option3, etc. = string.split.  If I am not mistaken, you were looking at just making the statement an assignment automatically (N=This is the text for the note).  While this may work in some places, it would not work in every case because of the options.  Since you would have to split those anyway, it might be easier to just split them to begin with.

Ludovicus_Maior's picture

Splits

Ah, now I see what you mean.   Then I would be tempted to counter with:

T;Curing The Incurable|QID;13040;13041;13042|N;Loot Venom Sacs off the Nerubians.|S

with an implicit STEP; at the start of the line.   The parse becomes:

tags=split("STEP;"+line.strip(),"|")

each tag could then be split up using:

tag=split(tags[i],";")

 

Silvann's picture

I see what you mean and I

I see what you mean and I support that!

Jiyambi's picture

Love these ideas! The currect

Love these ideas! The currect syntax was developed VERY haphazardly - first I took what syntax already existed for Tour Guide and modified it, then built on it tag by tag as the issue arose or as features were thought of. I also had ZERO experience programming so I wasn't really thinking about the rammifications of the syntax itself, just about how to get that next feature working.

I would love to preserve the short style of the tags so that the lines remain easily human-readable, even for non-programmers. With that caveat, I definitely support this and would be willing to help out if needed, though I still can't commit much time to the project until this summer.