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

Added XML repair functionality to fix #14 #16

Closed
wants to merge 6 commits into from
Closed

Added XML repair functionality to fix #14 #16

wants to merge 6 commits into from

Conversation

JyeGuru
Copy link
Contributor

@JyeGuru JyeGuru commented May 22, 2014

No description provided.

@PeridexisErrant
Copy link

The appveyor build seems to have included the 7z libraries and the readme... but not the Legends Viewer executable.

@JyeGuru
Copy link
Contributor Author

JyeGuru commented May 23, 2014

Not sure how the AppVeyor settings were set up for Parker147's account, but it was outputting ok on mine (using a powershell script). I have added 'appveyor.yml' config file that will ensure AppVeyor builds and outputs it the same. Not saying these are the best settings - just that they work - happy for them to be changed.

In addition to the normal executable, I specifically added a zip output to the PS script, this adds all the files that are found in the DFFD download of LegendsViewer (the DLLs, exe, readme) rather than including everything (eg: the PDB file).

XML.WhitespaceHandling = WhitespaceHandling.Significant;
}

public static string SafeXMLFile(string xmlFile)
Copy link
Owner

Choose a reason for hiding this comment

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

This function is unnecessary. We should just be catching the exceptions thrown by our own Parse function instead of the XDocument.Load. If a file doesn't have an error it has to be parsed twice, once by XDocument and then once by our own XMLParser. XDocument is way less intense, so it may be negligible for smaller files, but I feel like it would still add some time for bigger files.

@Parker147
Copy link
Owner

Yeah I played around with AppVeyor since I saw that you were using it and it looked nice, but didn't take the time to figure out how I could make it package all the build artifacts for me. That yml is a good addition.

@JyeGuru
Copy link
Contributor Author

JyeGuru commented May 25, 2014

With regards to the function - I split it away from the main handing so it can be modified/improved if any other common XML issues come up. I tried my code on the largest export I had at hand (~450Mb uncompressed) and it didn't seem to have a significant impact on the load time, however this was only a basic test using a couple of PCs I have access to.

Give me a few and I'll move the try/catch around the new World() bit.

@Parker147
Copy link
Owner

Merged a86a81c, 16f5e93

@Parker147 Parker147 closed this May 30, 2014
figment pushed a commit to figment/Legends-Viewer that referenced this pull request Jan 30, 2016
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.

None yet

3 participants