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

Parser too strict (and maybe buggy) #502

Closed
Speciesx opened this Issue Dec 23, 2015 · 5 comments

Comments

Projects
None yet
2 participants

@only-a-ptr only-a-ptr self-assigned this Dec 29, 2015

@only-a-ptr only-a-ptr added the coding label Dec 29, 2015

@only-a-ptr only-a-ptr changed the title from Parser errors to Parser too strict Dec 29, 2015

@only-a-ptr only-a-ptr added bug and removed coding labels Dec 29, 2015

@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Dec 29, 2015

Member

Many many syntax flaws in the truckfile....
And super-duper strict parser...

23:17:51:  == Parsing vehicle file: volvo-EC140C.truck
#FF3300 ERROR #FFFFFF (Section contacters)
    Line (1966): 205,
    Message: Invalid line, ignoring...

The trailing comma is invalid. But old parser would just ignore it.
EDIT : Fixed in upstream: f3d33b1

23:18:11:  == Parsing vehicle file: volvofmx-6x6withcrane.truck
#FF3300 ERROR #FFFFFF (Section torquecurve)
    Line (46):  NOTGGLE
    Message: Invalid mode keyword, ignoring whole line...

NOT!o!GGLE... I'm not sure what old parser would do here... I guess it would just fall back to defaults.
EDIT: Right, it would just ignore the option:

} else if (sourceStr == "NOTOGGLE" || sourceStr == "notoggle" || sourceStr == "Notoggle" || sourceStr == "NoToggle")
{
alb_notoggle = true;
}

EDIT : Fixed in upstream (TractionControl): 6b8b922

#FF3300 ERROR #FFFFFF (Section rotators_2)
    Line (1867): 222,217,223,225,226,224,218,220,
                 221,219,0.1,9,10,1000000,0.1   turning-grapple
    Message: Invalid line, ignoring...

Hmm, probably the parser is too strict on commas versus spaces here..
EDIT: Right, the parser requires a comma before the 'description'. Will loosen it up...
EDIT : Fixed in upstream: e6b80c5

#FFFF00 WARNING #FFFFFF (Section ties)
    Line (1967): 222,3,1,1,1,n,0,1
    Message: Ignoring invalid option: ,

A bug... a comma should always be treated like a delimiter, never as an option
Fixed, pullrequest: #674

#FF3300 ERROR #FFFFFF (Section hooks)
    Line (1983): selflock
    Message: Invalid option of 'hooks', ignoring...

Ignoring what? It's an error, so probably whole line. I doubt old parser would be that strict
EDIT: Ulteq pointed me out to the issue...#502 (comment)
Fixed, pullrequest: #674

#FF3300 ERROR #FFFFFF (Section flares2)
    Line (2010): 5,4,1,-0.61,0.3,0.0,b -1,0,-1, default
    Message: Invalid line, ignoring...

Dumb super-strict parser (comma vs. space separators), this is already reported elsewhere.
EDIT : Fixed in upstream: 3c85ad2

#FF3300 ERROR #FFFFFF (Section props)
    Line (2043): 137,135,141, ,0,0,0 ,90,0,0 ,vlvfmx48cabinS.mesh
    Message: Invalid line, ignoring...

OK, this is an actual truckfile error: {141, ,0} But IIRC old parser handled this fine.
Fixed, pullrequest: #674

#FF3300 ERROR #FFFFFF (Section flexbodies)
    Line (2199): forset256,255,254,253
    Message: Invalid line, ignoring...

Truckfile error again, no space between forset and the number. But I guess the old parser would just ignore the '256' and happily process the rest. Needs investigation.
EDIT: Turns out old parser accepted all the numbers fine
Fixed, pullrequest: #674

Member

only-a-ptr commented Dec 29, 2015

Many many syntax flaws in the truckfile....
And super-duper strict parser...

23:17:51:  == Parsing vehicle file: volvo-EC140C.truck
#FF3300 ERROR #FFFFFF (Section contacters)
    Line (1966): 205,
    Message: Invalid line, ignoring...

The trailing comma is invalid. But old parser would just ignore it.
EDIT : Fixed in upstream: f3d33b1

23:18:11:  == Parsing vehicle file: volvofmx-6x6withcrane.truck
#FF3300 ERROR #FFFFFF (Section torquecurve)
    Line (46):  NOTGGLE
    Message: Invalid mode keyword, ignoring whole line...

NOT!o!GGLE... I'm not sure what old parser would do here... I guess it would just fall back to defaults.
EDIT: Right, it would just ignore the option:

} else if (sourceStr == "NOTOGGLE" || sourceStr == "notoggle" || sourceStr == "Notoggle" || sourceStr == "NoToggle")
{
alb_notoggle = true;
}

EDIT : Fixed in upstream (TractionControl): 6b8b922

#FF3300 ERROR #FFFFFF (Section rotators_2)
    Line (1867): 222,217,223,225,226,224,218,220,
                 221,219,0.1,9,10,1000000,0.1   turning-grapple
    Message: Invalid line, ignoring...

Hmm, probably the parser is too strict on commas versus spaces here..
EDIT: Right, the parser requires a comma before the 'description'. Will loosen it up...
EDIT : Fixed in upstream: e6b80c5

#FFFF00 WARNING #FFFFFF (Section ties)
    Line (1967): 222,3,1,1,1,n,0,1
    Message: Ignoring invalid option: ,

A bug... a comma should always be treated like a delimiter, never as an option
Fixed, pullrequest: #674

#FF3300 ERROR #FFFFFF (Section hooks)
    Line (1983): selflock
    Message: Invalid option of 'hooks', ignoring...

Ignoring what? It's an error, so probably whole line. I doubt old parser would be that strict
EDIT: Ulteq pointed me out to the issue...#502 (comment)
Fixed, pullrequest: #674

#FF3300 ERROR #FFFFFF (Section flares2)
    Line (2010): 5,4,1,-0.61,0.3,0.0,b -1,0,-1, default
    Message: Invalid line, ignoring...

Dumb super-strict parser (comma vs. space separators), this is already reported elsewhere.
EDIT : Fixed in upstream: 3c85ad2

#FF3300 ERROR #FFFFFF (Section props)
    Line (2043): 137,135,141, ,0,0,0 ,90,0,0 ,vlvfmx48cabinS.mesh
    Message: Invalid line, ignoring...

OK, this is an actual truckfile error: {141, ,0} But IIRC old parser handled this fine.
Fixed, pullrequest: #674

#FF3300 ERROR #FFFFFF (Section flexbodies)
    Line (2199): forset256,255,254,253
    Message: Invalid line, ignoring...

Truckfile error again, no space between forset and the number. But I guess the old parser would just ignore the '256' and happily process the rest. Needs investigation.
EDIT: Turns out old parser accepted all the numbers fine
Fixed, pullrequest: #674

@only-a-ptr only-a-ptr changed the title from Parser too strict to Parser too strict (and maybe buggy) Dec 29, 2015

@Speciesx

This comment has been minimized.

Show comment
Hide comment
@Speciesx

Speciesx Dec 29, 2015

Contributor

@only-a-ptr
http://www.rigsofrods.com/threads/112719-volvo-EC140C
This is the correct excavator.

WARNING (Section nodes)
Line (465): 334, 3.031496, 0.424640, 2.897959, c l 6000
Message: Failed to parse using safe method, falling back to classic method.

WARNING (Section nodes)
Line (465): 334, 3.031496, 0.424640, 2.897959, c l 6000
Message: Node has load-weight-override value specified, but option 'l' is not present. Ignoring value...

Contributor

Speciesx commented Dec 29, 2015

@only-a-ptr
http://www.rigsofrods.com/threads/112719-volvo-EC140C
This is the correct excavator.

WARNING (Section nodes)
Line (465): 334, 3.031496, 0.424640, 2.897959, c l 6000
Message: Failed to parse using safe method, falling back to classic method.

WARNING (Section nodes)
Line (465): 334, 3.031496, 0.424640, 2.897959, c l 6000
Message: Node has load-weight-override value specified, but option 'l' is not present. Ignoring value...

@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Dec 30, 2015

Member

@Speciesx Flawed syntax, it wouldn't work correctly in the old parser either

Line (465): 334, 3.031496, 0.424640, 2.897959, c l 6000

There's a space (valid separator) between c and l, which means syntax check fails and parser falls back to "classic method (0.x3 compatible)":

  • c is 'options'
  • l is 'load weight override' (not a number -> parsed as 0 for compatibility)
  • 6000 is invalid trailing text, silently ignored for compatibility
Member

only-a-ptr commented Dec 30, 2015

@Speciesx Flawed syntax, it wouldn't work correctly in the old parser either

Line (465): 334, 3.031496, 0.424640, 2.897959, c l 6000

There's a space (valid separator) between c and l, which means syntax check fails and parser falls back to "classic method (0.x3 compatible)":

  • c is 'options'
  • l is 'load weight override' (not a number -> parsed as 0 for compatibility)
  • 6000 is invalid trailing text, silently ignored for compatibility
@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Dec 30, 2015

Member

Note to self:

  • improve the warning message, it should say "Syntax check failed, falling back to classic lenient parsing."
  • add data breakdown for verification. It should say:
    • ..snip..
    • Options: c_CONTACTLESS (Input text: "c")
    • Load weight override: 0 (input text: "l")
    • Invalid trailing text: "6000"

EDIT: fixed in upstream: f91373f

Member

only-a-ptr commented Dec 30, 2015

Note to self:

  • improve the warning message, it should say "Syntax check failed, falling back to classic lenient parsing."
  • add data breakdown for verification. It should say:
    • ..snip..
    • Options: c_CONTACTLESS (Input text: "c")
    • Load weight override: 0 (input text: "l")
    • Invalid trailing text: "6000"

EDIT: fixed in upstream: f91373f

@only-a-ptr only-a-ptr added enhancement and removed enhancement labels Dec 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment