Further Code Clean Up #12

MartinX3 · 2017-08-07T22:05:13Z

Added ToDo's

Fixed moons-String.

Optimized Code further (C#7 conventions)

Changed some code to LINQ

Sync with Master

Added ToDo's

Fixed moons-String. Added ToDo's

Changed some code to LINQ

StollD · 2017-08-07T23:43:26Z

Source/Database/PQSPreset.cs

 namespace Stellarator.Database
 {
+    using ConfigNodeParser;


Please leave the usings outside of the namespace declaration.

StollD · 2017-08-07T23:44:07Z

Source/Database/PQSPreset.cs

    /// <summary>
-    /// Prefab for a PQS
+    ///     Prefab for a PQS


I am not sure if I like these leading whitespaces.

Should I revert this?
I thought with them, the text between the next nodes is easier to read. :)

StollD · 2017-08-07T23:45:13Z

Source/Generator.cs

        /// </summary>
-        public static Int32 Seed { get; set; }
+        private static int Seed { get; set; }


Please use CLR type names instead of type aliases

StollD · 2017-08-07T23:45:35Z

Source/Generator.cs

        {
            // Log
            Console.WriteLine("Generating the solar system...");
            Console.WriteLine();

            // Create a new Solar System using libaccrete
-            SolarSystem system = new SolarSystem(false, true, s => { });
-            SolarSystem.Generate(ref system, seed.GetHashCode(), Int32.MaxValue);
+            var system = new SolarSystem(false, true, s => { });


Please don't use var.

StollD · 2017-08-07T23:47:35Z

Source/Generator.cs

                else
                {
                    template.AddValue("name", Utility.GetTemplate(planet.surface_pressure > 0.00001, false));
-                    template.AddValue("removeAllPQSMods", "True");
+                    template.AddValue("removeAllPQSmods", "True");


You know that you changed a value that is later read by Kopernicus? Kopernicus is case sensitive

StollD · 2017-08-07T23:49:05Z

Source/Generator.cs

-                Color average = default(Color);
-                GeneratePQS(ref node, name, folder, planet, ref average);
+                Color average;
+                GeneratePqs(ref node, name, folder, planet, out average);


Please keep it PQS

StollD · 2017-08-07T23:49:44Z

Source/Generator.cs


            // Create a node for the mods
-            ConfigNode mods = new ConfigNode("Mods");
+            var mods = new ConfigNode("mods");


Same as above, you changed a value that is later read by Kopernicus

StollD · 2017-08-07T23:51:29Z

Source/Generator.cs

+                        {
+                            // Create a VertexBuildData
+                            var builddata = new VertexBuildData
+                                            {


This indent looks weird

ReSharper told it to me, for a better readability
Should i revert it?

StollD · 2017-08-07T23:52:48Z

Source/Generator.cs

+                    // Save the textures
+                    Directory.CreateDirectory(Directory.GetCurrentDirectory() + "/systems/" + folder + "/PluginData/");
+                    using (var normals = Utility.BumpToNormalMap(height, 9)
+                    ) // TODO: Implement something to make strength dynamic


No newline maybe?

The newline intends a better reading
Should I revert it?

diffuse.Bitmap .Save(Directory.GetCurrentDirectory() + "/systems/" + folder + "/PluginData/" + name + "_Texture.png", ImageFormat.Png);

In the code I marked, there is a new line for a ), I don't think that is neccessary. The comment can stay on the new line though.

Ahh, It is changed. :)

"Please leave the usings outside of the namespace declaration." in Database "Please use CLR type names instead of type aliases" "Please don't use var." "You know that you changed a value that is later read by Kopernicus? Kopernicus is case sensitive" D: Sorry "Please keep it PQS" "Same as above, you changed a value that is later read by Kopernicus" D: Still sorry D:

MartinX3 · 2017-08-17T22:35:02Z

I made several commits to integrate your advises
I look how I can merge them with this pull request.

Edit:
Oh, they are automatically integrated. Nice.
@StollD may you look at this?

StollD · 2017-08-18T10:37:43Z

Source/Program.cs

-            var seed = Prompt("Please enter the seed you want to use: ", "--seed");
-            var folder = Prompt("Please choose a folder name for your system: ", "--name");
-            var systematic = Prompt("Use systematic planet names? (y/n) ", "--systematic", true);
+            string seed = Prompt("Please enter the seed you want to use: ", "--seed");


Please use CLR type names here too (and everywhere else)

MartinX3 · 2017-08-19T20:27:57Z

Hey @StollD , is that okay now? :)

…t is neccessary. The comment can stay on the new line though." - Added a info about the code conventions.

MartinX3 · 2017-08-20T17:24:11Z

I also added an Info about the code conventions, you want in this project. :)

StollD · 2017-08-20T19:47:58Z

Looks ok to me. Merging.

Thanks for your contribution!

MartinX3 · 2017-08-20T19:49:18Z

Perfect :)
And I'm glad, that you are happy about my contribution :D

MartinX3 and others added 5 commits August 6, 2017 14:10

Merge pull request #1 from Kopernicus/master

3fd90d5

Sync with Master

Optimized Code (C#7)

361ba00

Added ToDo's

Optimized Code (C#7)

e203b04

Added ToDo's

Optimized Code further (C#7)

459c03e

Fixed moons-String. Added ToDo's

Optimized Code further (C#7 conventions)

3f27baa

Changed some code to LINQ

StollD suggested changes Aug 7, 2017

View reviewed changes

StollD reviewed Aug 18, 2017

View reviewed changes

"Please use CLR type names here too (and everywhere else)"

898d603

"In the code I marked, there is a new line for a ), I don't think tha…

756ed3b

…t is neccessary. The comment can stay on the new line though." - Added a info about the code conventions.

StollD approved these changes Aug 20, 2017

View reviewed changes

StollD merged commit 7dccb0d into Kopernicus:master Aug 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Further Code Clean Up #12

Further Code Clean Up #12

MartinX3 commented Aug 7, 2017

StollD Aug 7, 2017

StollD Aug 7, 2017

MartinX3 Aug 17, 2017

StollD Aug 7, 2017

StollD Aug 7, 2017

StollD Aug 7, 2017

StollD Aug 7, 2017

StollD Aug 7, 2017

StollD Aug 7, 2017

MartinX3 Aug 17, 2017

StollD Aug 7, 2017

MartinX3 Aug 17, 2017 •

edited

Loading

StollD Aug 18, 2017

MartinX3 Aug 20, 2017

MartinX3 commented Aug 17, 2017 •

edited

Loading

StollD Aug 18, 2017 •

edited

Loading

MartinX3 commented Aug 19, 2017

MartinX3 commented Aug 20, 2017

StollD commented Aug 20, 2017

MartinX3 commented Aug 20, 2017

Further Code Clean Up #12

Further Code Clean Up #12

Conversation

MartinX3 commented Aug 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinX3 Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinX3 commented Aug 17, 2017 • edited Loading

StollD Aug 18, 2017 • edited Loading

Choose a reason for hiding this comment

MartinX3 commented Aug 19, 2017

MartinX3 commented Aug 20, 2017

StollD commented Aug 20, 2017

MartinX3 commented Aug 20, 2017

MartinX3 Aug 17, 2017 •

edited

Loading

MartinX3 commented Aug 17, 2017 •

edited

Loading

StollD Aug 18, 2017 •

edited

Loading