ConsoleView and file path manipulation improvements #66
Conversation
Does not fix high performance cost
| @@ -50,11 +52,10 @@ public MainWindow() | |||
|
|
|||
| Settings.PropertyChanged += SettingChanged; | |||
| SettingPanel.Settings = Settings; | |||
| if (!File.Exists(Directory.GetCurrentDirectory() + @"\Images\" + Settings.CurrentMapFile)) | |||
| if (!File.Exists(Path.Combine(Directory.GetCurrentDirectory(), "Images", Settings.CurrentMapFile))) | |||
There was a problem hiding this comment.
Crashes when Settings.CurrentMapFile is null (the case when any new computer runs the program) or all map data is cleared.
| public void DisplayMap(string mapSetFile) | ||
| { | ||
| ClearCanvas(); | ||
| // load in individual images | ||
| // TODO get the path from the settings | ||
| if (File.Exists(Directory.GetCurrentDirectory() + @"\Images\" + mapSetFile)) | ||
| string path = Path.Combine(Directory.GetCurrentDirectory(), "Images", mapSetFile); |
There was a problem hiding this comment.
Crashes here is mapSetFile is null
There was a problem hiding this comment.
Why would mapSetFile be null? It's provided as a parameter. DisplayMap shouldn't be called if the file is null.
There was a problem hiding this comment.
In updateMapWaypoints in MainWindow.xaml.cs, DisplayMap is called without regarding the nullity of the mapSetFile.
| @@ -5,7 +5,7 @@ | |||
| <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> | |||
| <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> | |||
| <ProjectGuid>{717939F0-9BDC-4D66-99B5-AE34D586488E}</ProjectGuid> | |||
| <OutputType>WinExe</OutputType> | |||
| <OutputType>Exe</OutputType> | |||
There was a problem hiding this comment.
You can't just convert this to a console application! We still need the UI to spawn in which is only does as a Windows Application. You'll have to spawn the command prompt as a child process and redirect standard out to it. If you leave it as a Windows Application and have no command process it will write to Visual Studio console, but we can't assume Visual Studio will always be running.
There was a problem hiding this comment.
The UI still runs when it's a console application.
There was a problem hiding this comment.
It wasn't running on my computer when I tested it. I'll try again tonight!
|
I kinda feel like refactoring a bunch of the map config stuff. It's probably just because I'm unfamiliar with the code, but the control flow of how things are initialized and what values different fields are supposed to have is driving me insane. |
|
There's a bug affecting me right now that makes Directory.GetCurrentDirectory() return "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE". There is a bug report at microsoft/vstest#311 but it says it's fixed and I don't know if the version it is fixed for has been released or not. When is " VS 2017 update 1" supposed to be released? |
|
I see what you mean about the initialization, it would help if at least the method parameters were well documented, open a new pull request if you wish to do this, but I'm not sure if we should this close to competition. Also this bug isn't effecting me, see if there are updates in the Visual Studio Installer. |
|
I am closing and merging this because the console was corrected, I am reverting your Path.Combine on the 2 crashing areas rather than refactoring. |
I haven't tested the file path stuff much yet. Will resolve #61.