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

Project Restructure #31

Closed
hexabits opened this issue Feb 20, 2014 · 12 comments · Fixed by #43
Closed

Project Restructure #31

hexabits opened this issue Feb 20, 2014 · 12 comments · Fixed by #43

Comments

@hexabits
Copy link
Member

[WIP]

nifskope
+---build
|   *.bat
|   *.in
|   *.sh
+---dep
|   *.dll
+---docsys @ cf44594
+---include [Optional?]
+---install
|   +---linux-install
|   +---win-install
+---lib
|   +---fsengine
|   +---NvTriStrip
|   +---qhull @ 60d5581
+---res
|   +---icon [Move current resources to icon subdir?]
|   +---lang
|   |   *.ts
|   +---shaders
|   |   *.frag
|   |   *.prog
|   |   *.vert
|   +---ui
|   |   *.ui
|   *.ico
|   *.png
|   *.qrc
|   *.qss
|   *.rc
+---src
|   +---gl
|   +---importex
|   +---spells
|   +---ui
|   +---widgets
|   *.cpp
+---test
|   .gitattributes
|   .gitignore
|   .gitmodules
|   CHANGELOG.TXT [Change to GitHub Readme format?]
|   LICENSE.TXT [Change to GitHub Readme format?]
|   TODO.TXT [Legacy?]
|   VERSION [Put in \build instead?]

I put some comments in brackets. To elaborate:

  • Icon PNGs currently in resources should possibly be moved to their own subdir res\icons
  • Include directory optional? NifSkope.pro might be sufficient in giving anything under lib a public interface.
  • Change TXT files to GitHub readme format?
  • Remove TODO? Take anything useful and turn it into an issue.
  • Put VERSION in build? Depends on if the .PRO should go in build or the root.

Changes will need to be reflected in:

  • NifSkope.pro
    • Change paths to source, resources, headers, libs
    • If using separate include dir, add to the path so header includes can remain simple
  • .gitmodules - The submodules will need their "path" value changed

Tracking

Refactor Progress #32

@neomonkeus
Copy link
Member

Looks good, would it be overkill to nest all the code related folders in one master source folder?

@hexabits
Copy link
Member Author

I dunno. I'm ambivalent as to whether the headers are separated into their own include dir. However I think that the separation of lib and src is necessary. If they're not really relevant to our source code, and shouldn't be intermixed, like qhull, they shouldn't be in the same parent source directory but instead both in the project root. Especially if we will create libs with their own .PRO files that we build as static dependencies.

For example qhull is like an entirely separate project with its own project structure. I considered listing docsys there too. Fsengine and NvTriStrip are borderline. As of now FSEngine relies on the project for Qt, etc. But it's reasonably detached from the rest of the project's code. And in the future we might refactor FSEngine entirely since it can no longer work as-is under Qt 5.2.

Basically anything we separate into its own config (e.g. CONFIG += fsengine) scope in NifSkope.pro should probably be placed under libs. I don't think src/src and src/libs really makes as much sense.

@neomonkeus
Copy link
Member

Sorry should clarify, its not necessarily a source folder, just a general parent folder.

@hexabits
Copy link
Member Author

I dunno. I think lib, src, include are extremely common for the root dir. If anything just remove the dep folder and merge that into lib. For example NifMopp.dll... is there a project for this somewhere with source? The source could instead be included at lib, and a .PRO file could be generated to allow us to build NifMopp dynamically or statically.

I wouldn't know what to call it as the libs aren't our source.

If include is used for anything I guess it should be the public interface for anything inside of lib.

@neomonkeus
Copy link
Member

Makes sense. I think it just because I am looking at the current layout and its so cluttered, the proposed structure is perfect, just nitpicking :P

@hexabits
Copy link
Member Author

Is there source for NifMopp.dll though? Best I can find is this: https://github.com/Ethatron/nifopt/blob/master/nifopt-hvk.C

The DLL didn't come out of nowhere, and it's not directly from a vendor. I would prefer we have the ability to rebuild such dependencies if we needed to. Especially if we move on to newer compilers. I can't tell you offhand if the DLL would cause issues, but it's probable.

Otherwise we would possibly have to force someone to use an old version if they want to update any MOPP code.

@neomonkeus
Copy link
Member

Looking about I found this commit -
added NifMopp.dll from niftools revision 4012 - (trunk/contrib/niflib/NifMopp/NifMopp.dll).
This would suggest that its an external dependency, built via Niflib, Thankfully Skyfox has kept it up to date.

Correction, separate project we created. https://github.com/niftools/mopper.
Again see Skyfox's fork for any updates.

@hexabits
Copy link
Member Author

Nah that's not quite it. It's going to be a file with these functions:

GenerateMoppCode
RetrieveMoppCode
RetrieveMoppScale
RetrieveMoppOrigin
GenerateMoppCodeWithSubshapes

And that nifopt-hkv.c file definitely fits the bill when it comes to having the proper methods.

Both Mopper and that seem to be for standalone executables though. The nifopt-hvk.c file being for Ethatron's Nifopt obviously.


Edit: Just noticed the license in the header of that nifopt-hvk.c file:

Copyright (c) 2006-2008, NIF File Format Library and Tools.

So it must be from somewhere... It just looks to be adapted around Nifopt in Ethatron's case.

@neomonkeus
Copy link
Member

Yeah I think Mopper is the version used by pyffi for it Mopp code generation. I would suggest trying to contact either of the two above and see what they know. I know Skyfox had to use a newer version of the Havok SDK for the bhkCompressedMesh generation, but had decoded things to the point the he could write an open source version. He is currently however occupied with work given his post on the forum, but may still respond with general information.

@hexabits
Copy link
Member Author

hexabits commented Mar 4, 2014

Issues (Updated)
  1. README.txt.in is basically completely outdated.
  2. doxygen_makeapidocs.bat and doxygen_makeqttag.bat have hardcoded doxygen executables instead of relying on doxygen being on the path. The latter also assumes a very specific Qt version and path (same goes for the .sh variant).
  3. Doxyfile.in needs paths adjusted, and hardcoded Qt version references removed.
  4. All the files in /build and /install need their paths adjusted for the most part.
  5. sfannounce.txt ... Turn into README.md for the Github repo main page.
  6. Should fix up the .gitignore while I'm in there changing everything. :)
  7. For the TXT files which get included in the install... Relocate to one of:
    • build/
    • install/
    • res/
      There are rationalizations for each of these. Some files in res/ are there to be included in the release (lang, shaders). Putting it in the root of install/ makes some sense, because they are files common to both linux/win. Putting it in build/ could make sense as they may get cat/sed or otherwise parsed as part of a build process.
      _OR..._ Leave them in place, also convert to .md for easy viewing from the repo. Generate TXT files from these at build time.
  8. Relocate docsys to build directory? It's only meant to be a pre/post link build step, and has no bearing on the actual source like a lib/dep would.
  9. Relocate VERSION to `build directory?
  10. Relocate .pro/.pri files to build directory? Or should the only thing in the root be qmake/make files, aside from TXT and .git files?

@neomonkeus
Copy link
Member

Re point no. 8
QString fname = dir.filePath( "nif.xml" ); // last resort // Try local copy first, docsys, relative from nifskope/release, relative from ../nifskope-build/release, linux data dir. QStringList xmlList( QStringList() << "nif.xml" << "docsys/nifxml/nif.xml" << "../docsys/nifxml/nif.xml" << "../../docsys/nifxml/nif.xml" << "../nifskope/docsys/nifxml/nif.xml" << "../../nifskope/docsys/nifxml/nif.xml" << "/usr/share/nifskope/nif.xml" );

@hexabits
Copy link
Member Author

hexabits commented Mar 5, 2014

I don't like that hardcoded stuff at all actually. I already took out some code that changes the directory to search for the shaders folder if the EXE is in a folder named "release" or "debug". hexabits/nifskope@d5875fa3

All that is for is just searching for nif.xml when the application starts. I'm not sure NifSkope should look for the docsys folder at all. It's relevant only to dev builds. I also copy nif.xml to the build folder along with kfm.xml, the lang and shaders folders, and the necessary DLLs (if a non-static build).

Actually that code is only useful if you build NifSkope underneath the project directory, which qmake doesn't like (that's why they're called shadow builds). And even if you wanted to, the qmake script would place the nif.xml alongside the exe.


I could technically leave the /usr/share bits alone... But I don't like that they're there at all. I could leave them in and wrap them with an #ifdef so that it only compiles on Linux. The relative paths I don't see the point in.

@hexabits hexabits mentioned this issue Mar 10, 2014
@neomonkeus neomonkeus added this to the v1.1.4 milestone Mar 11, 2014
@hexabits hexabits modified the milestones: v1.2.0, v1.1.4 Mar 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants