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

Change: Improve number parsing #544

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Change: Improve number parsing #544

wants to merge 11 commits into from

Conversation

leezer3
Copy link
Owner

@leezer3 leezer3 commented Oct 7, 2020

This branch attempts to unify & improve the number parsing code.

Most places parsing a Vector simply called the NumberFormats.TryParseDoubleVb6 method repeatedly on each component of the vector, and had their own Interface.AddMessage call with an appropriate error message.

This PR creates several new helper functions in the NumberFormats file which perform the function in a unified way.
It also adds an unconditional number parser, which adds an appropriate error,

Benefits:

  • Single set of methods for parsing vectors.
  • Gets rid of a bunch of duplicated code.

Drawbacks / Issues

  • This unfortunately changes the structure of some error messages. Most users don't worry about these, but may confuse developers slightly?
    New: Error X is invalid in AddVertex - Coordinates at line 2 in B:\test\FCMB 2000\3D\Cabina\AgullaPressioB.csv
    Old: Error Invalid argument vX in AddVertex at line 2 in file B:\test\FCMB 2000\3D\Cabina\AgullaPressioB.csv
  • Should these be moved to be vector constructors?
  • Slightly nasty hack to enable the messages to be added by the API.
  • More places can use this, e.g. TrainEditor2
  • Some bits in the panel parsers could use Vector2 more often.

TEST

  • Are there any places where an array length check is now required?
  • Potentially dicey code in the Panel.xml parser- Some angles don't appear to be converted to radians. Double check what's going on here....

@s520 any thoughts?
Ended up running this up as I looked in the Panel2 parser in preparation for adding the windscreen properties & decided I could get rid of some of the duplicated code.

@leezer3 leezer3 changed the title Change: Unify vector parsing Change: Improve number parsing Oct 8, 2020
{
Interface.AddMessage(MessageType.Error, false, "ValueInDegrees is invalid in " + Key + " in " + Section + " at line " + LineNumber.ToString(Culture) + " in " + FileName);
}
InitialAngle = NumberFormats.ParseDouble(Value, Key, Section, LineNumber, FileName);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be converted to radians?
Nasty suspicion it should....

@s520
Copy link
Contributor

s520 commented Oct 9, 2020

I'm busy this week, so I'd like to see it next week.

@leezer3
Copy link
Owner Author

leezer3 commented Oct 9, 2020

No trouble.

There are a lot more places and similar bits that can be added (color parser for one, used in the objects, panels etc), & I'd consider this low priority cleanup work at best.

@leezer3
Copy link
Owner Author

leezer3 commented Oct 29, 2020

This PR is now about 50% of the way towards replacing the parse calls.

A lot of the rest of these use the negative response to set some sort of default value. (where the standard double / int default of 0.0 doesn't apply)
Need to think about the correct way to do this....

@leezer3 leezer3 force-pushed the master branch 3 times, most recently from 5f23a6d to a53fd14 Compare October 4, 2024 11:49
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.

2 participants