Closed h0rnman closed 8 years ago
Hey! I've actually made a few changes already thanks to another contributor, so we've done a few of these already, but is it ok if I implement your great checkbox and process validation? I'll add you to the readme as a thank you too if you like!
Not a problem - I just forked the most recent version on github, so I didn't catch any unsunchronized changes. That said, if you like the submission, steal whatever you want, I won't mind, I promise!
haha awesome, thank you! Did you want me to list you in the readme? I'm happy to link to somewhere for you (Github/portfolio/social) if you want?
That's fine - I'm just happy to contribute. If you want to link, I'm at https://github.com/h0rnman and https://veryposh.wordpress.com/
Just added you in, thanks again!
No problem at al - glad I could help. I'm curious, though. why the revert to text-box for delay and time and why revert back to the messy way of generating the parameters list? I still see a lot of places where things could be improved, but if you're heading in a different direction, I want to avoid making changes that will just get reverted.
Another contributor is currently re-writing the bencher application in C# so that its a forms application, as well as making other changes to the launcher side of things, so until it's updated (which should be today, perhaps tomorrow) I'd probably recommend holding off on updates. He just sent over some of the current changes he's doing, I'll list below.
There are comments under launch_Click under the first check that explains what’s happening here. d. launch_Click i. Adjusted the logic here to use tryParse to protect against invalid input. (The comments have more details). ii. Added return to the SelectedIndex == -1 check to prevent needless execution of code that will not succeed. iii. Added a new catch statement to the try for the Process.Start to catch miscellaneous exceptions (I’m doing this until I’m sure about the implementation). This is me being pedantic. You can probably safely remove this.
Ok - sounds good to me. I've been in contact with Jim about a Powershell script I wrote to parse the CSV files and turn them into charts. It uses almost entirely native C# code (not Powershell built-ins) so it should be almost a direct copy/paste. I'll hold off on things until the next check-in and see if I can add anything else.
Ah awesome, I think Jim passed that on so I'll see if we can implement, although we did make a few changes to how the AVG FPS was calculated, as well as how the output file is saved and what else is saved in it - so might need to alter that too. But thanks again for the help already, and I'll let you know when the application and files are updated!
Hey man, just made all the changes and are live with this version now. I'm sure there are better ways to do some of the things here, but we've tested these methods quite a lot and seem to work fine for now - if you have any drastic improvements then feel free to go for it - but otherwise it's probably more feature adds now. By all means take a look though!
No longer needed - closing
Updated argument list construction Code cleanup Added validation for process selection Updated the checkbox handling so that checking then unchecking doesn't result in the same parameter being added twice (like " -simple -simple") Changed Delay and Time boxes from textBox to numericalUpDown to remove Convert directives