Pitt-RAS / micromouse-2016

7 stars 6 forks source link

Parser should use our new Queue class instead of FakePath #114

Open awglyde opened 7 years ago

awglyde commented 7 years ago

It looks like FakePath is wrapping the new Queue class. If we swap the FakePath class with the Queue class we can simplify some of the parser code.

@alexanderbenfox, you've worked on the Parser a lot. What do you think?

alexanderbenfox commented 7 years ago

the FakePath class only exists for debugging purposes. before we swapped out the queues, it served a purpose in being an intermediary between the path provided by the tester (as an array) and converted it to the path data type which the parser processes into a move list. if you can find a clean way to make the change, go ahead

awglyde commented 7 years ago

I thought it only existed for debugging purposes as well however I found that FakePath is being used to create the relative path in PathParser::buildRelativePath which is called by the PathParser constructor. That relative path goes on to be used by a plethora of functions to determine the move list.

ghost commented 7 years ago

@awglyde Oh yeah, FakePath does get used everywhere to build move_list. And it does look like pretty much a wrapper around Queue, but AFAIK it was meant to mimic a Path. Probably the reason a real Path wasn't used is that a Path has to be built from inside its own constructor, and that didn't work here?

Two options could be

  1. Replace FakePath with a plain Queue<Compass8, xx> like you said, or
  2. Attack #102, and then in theory that lets us use a plain Path instead of FakePath

Maybe it's even the case that in general, a Path should just be reduced to a Queue<Compass8, xx>. Or maybe a Queue doesn't even really fit the problem, maybe it should be basically an index-able array.

A bit of a tangle to sort out.

ghost commented 7 years ago

Also. This line is ill-formed, it shouldn't be 50*sizeof(Compass8). Queue capacity is the max number of elements that can be stored, not the number of bytes.