search

Boris-Barboris / AtmosphereAutopilot

Plugin for Kerbal Space Program
GNU General Public License v3.0
48 stars 16 forks source link

First draft on the new UI #4

Closed radistmorse closed 8 years ago

radistmorse commented 8 years ago

Here it is!

This commit adds a whole new project to the solution called AtmosphereAutopilot.GUI, which compiles as a separate dll. The changes to the AA itself are mostly located inside the nested class MainMenuController in the AtmosphereAutopilot class.

Other changes deal with the altitude and speed controllers, which were designed in such a way that it was impossible to change the values from outside.

With this patch both GUIs remain usable, but not synchronized: if you change values from the old GUI, new GUI remain unaffected. The update is done for the hotkeys though.

Now, the last interesting part is the prefab itself. Here, I just provide it in the BUILD folder, but to rebuild it yourself you'll need a unity editor installed, and a whole another project with all the sources and shit. Which I can provide, but I don't know where it would be best to do.

I also screwed your library paths. Which, to be honest, were screwed from the beginning. To test the new GUI you'll need both ddls and a prefab in the plugin root folder.

Boris-Barboris commented 8 years ago

Good to see you again.

I'll list some things I didn't like, but nothing very serious as far as I can see.

  1. Architecture of AtmosphereAutopilot class. You've put too much GUI detail inside a class, wich is supposed to perform very general form of memory managment, callback binding and dispatching. I suggest moving MainMenuController to separate file, making it a proper public\internal class and moving to it's methods as much as possible from AA class. Stuff like "private void mainMenuOpen()" should not be part of AtmosphereAutopilot highest-level entity.
  2. All files, that are used to build the mod must be a part of Git repo. Same goes for "AtmosphereAutopilot/icon.png" you deleted for some reason. Add icon.png back (preferably to VS solution as it was before), and add to repo (not to VS solution) all stuff, that is needed to build prefab (without unity tools themselves, of course, coder is expected to have them). Don't add build artifacts to repo (like prefab itself).
  3. If you make AA.dll hard-bound to new GUI stuff (and you do, judging by new AtmosphereAutopilot class code), there is zero point in having two projects.
  4. Questionable folder structure for new GUI stuff. No need for folder "GuiInterfaces" with one file in it, you can put it into GUI folder just right (assuming previous point was taken and it's all in one project at this point). For so-called GuiControllers (name ModuleGUI would fit better imho, so FlyByWireController -> FlyByWireGUI etc ) you should probably just create a subfolder in GUI folder. Utils can be a subfolder of GUI.
  5. Where did thrust control call go here: https://github.com/Boris-Barboris/AtmosphereAutopilot/pull/4/files#diff-4e254b583d875c855fea600eb572090cL98 ?
  6. Merge to "neo_gui" branch, I've just created it.

Good luck and thank you!

radistmorse commented 8 years ago

.1. Ok, I'll separate a class into a new file.

.2,3. The problem is, the unity project has 23 MB of auto-generated binary shit without which it won't work. But I think I found a way to distribute it more convenient.

.4. Well, renaming "...Controller" into "...GUI" is no biggy, but mixing both projects into the same folder is not good. They are two separate projects after all, just in the same solution. I'll put all the new stuff into the "NeoGUI" folder since that's how you want to name it.

.5. Whoops, my bad. I removed it while dealing with merge conflicts updating the stuff for a new version.

Boris-Barboris commented 8 years ago
  1. I'm pretty sure not all of those binary files are actually needed, many of those are caches. Visual Studio also has some of those, wich is reflected in gitignore. If you don't want to waste time finding out what are compulsory, just add all of them to git repo, it's not like we pity Github servers.
  2. I don't tend to think of a visual studio project as of one man's work. It's composition unit of a program. If you make two projects: autopilot itself and alternative gui, you create two things hard-dependent on each other, not in hierarchy (like service dll - client). AA is full of such things - classes. And they don't need to be separated in different dlls.
radistmorse commented 8 years ago

I was just composing another pull request to the new branch. There is a very important reason for separation, I'll explain it there.

radistmorse commented 8 years ago

And for the files, no, every one of them is important. As I said, unity project consists of 23 MB of weird files, and I spent several hours trying to strip it to bare minimum. I can return the .meta to gitignore, but the files in there are needed.