There’s a few choices of interest from winmain-and-main-in-c-extended, wonder if there is any point adding command line arguments say for example to run the show in debug mode like e.g. Wrye Bash.
Will that be for CMake? You can make it work with MSBuild as explained in CMAKE_MAKE_PROGRAM, not sure how that goes with VS on Linux though.
Are the wide characters versions still necessary? There’s been a lot of Unicode support added to Windows since then
Could be useful to add one to open a file from the command line. Mod managers could then easily let you choose to edit a file, for instance. Pretty easy to implement, would just need to brush up on the specific format. Can definitely add it to the wishlist. Can’t think of any others that would be helpful, can you?
Was thinking Meson since it is easier to read. It didn’t yet exist when I was working on BOSS but I do like what I’m reading about it. That’s definitely not set in stone, though
Got OpenMW mostly set up today for future testing. Morrowind itself (before OpenMW was installed as I had to generate the INI file) complained about it missing the plugin files even though they were enabled and had to disable them and enable them again before I could get it to launch. Still need to tweak the settings but it’s probably good enough for now. May try playing the game at some point. 😛
Edit:
cxxopts looks like a nicely done library for more complex command line arguments. Looked at gflags and Boost.Program Options and they were a bit more cumbersome to use
Edit 2:
A lot of the macros can probably be switched over to static const variables now that I think about it, going back to the earlier post about them.
Edit 3:
There are a lot of pragmas that deal with memory alignment. Since MWEdit doesn’t work with accessing memory, we may be able to remove them without any kind of ill effects. Will require research
Edit 4:
The conversion to constructors and destructors may be a bit cumbersome as the code heavily relies on custom create and destroy functions. That may be more easily done with a real IDE as opposed to a text editor so will put it on the back burner for now.
Sorry, no, meant the whole thread, not just that post. As a matter of interest, is MWEdit fully Unicode or MBCS?
If not for debugging, no, – a command line may not be absolutely mandatory unless an association is setup for opening files. Can you load more than one file in MWEdit?
Where does MWEdit store its preferences – in the registry or in a data/ini file? Have you seen this guide? It mentions the following:
Often you will remain with some errors not pointing to a explicit script name after Edit\Compile Active Scripts, e.g. something like “1) Warning: Line x (y): Comparison operator missing space characters on one/both sides!”
Problem is, line x may be ok, but instead of script name MWEdit spits a rather cryptic script index which I still haven’t deciphered.
In case this happens, it is often convenient to just use regular expression search from an external text editor to solve these pesky missing spaces errors.
One for the issues list?
Ah okay, whatever is best. 🙂
Ah, okay. So far, MWEdit appears to be ANSI so we’ll definitely want to do some testing with Unicode once we get a build together to see what support is like. 🙂
Haven’t gotten to the UI yet but I think it can only load one file at a time for editing.
Looks like it uses the registry for all of its settings. We may want to plan on adjusting it to use an external file for cross-platform support. INI files wouldn’t be a bad idea as they line up with what the community is already used to. Got a few libraries here, here, and here we can look at. Looks like there are also a few libraries in the PopOS repos that could be worth a look as well.
Good catch on the scripting bug. Reading through the issue list, it looks like it may be related to this one. Will get the additional details added!
Yeah, sorry for the rambling there! 😛
Edit:
esm/EsmSubNameFix.h has a random #if FALSE in it with its terminator at the end of the file. No condition other than if it’s false. No idea what that’s supposed to accomplish but I’m pretty sure it means none of the code will be loaded at all.
Edit 2:
This block in esm/EsmSubBase.h needs to be investigated:
operator dword(void) const {
return lType;
}
As far as I know, dword isn’t an operator but a Windows specific variable type but I don’t know much about the Windows version of the C/C++ runtime.
Edit 3:
Just finished reformatting the whitespace in the esm directory. Time to move to the next one!
Changes have been pushed upstream if you want to take a look at how much more readable the code is 🙂
Edit 4:
File/XML/ is now done. Will get it pushed with the rest of File/ later
Thanks. 🙂
With the esm header, good idea to check what’s in the cpp re anything relating.
With the X86 MSVC 19.42 compiler at godbolt there is:
source(2): error C2833: ‘operator dword’ is not a recognized operator or type
source(2): error C2059: syntax error: ‘newline’
source(2): error C2143: syntax error: missing ‘;’ before ‘{‘
source(2): error C2447: ‘{‘: missing function header (old-style formal list?)
The other three errors don’t occur with the latest X64 MSVC. Odd.
Yeah, that’s pretty normal. When there’s an error, oftentimes the compilers complain about surrounding lines as well even though there isn’t an error in them. Pretty annoying as it can sometimes be hard to fix a lot of issues all at once but I think it has something to do with how the parsers are designed.
Getting close to being finished with the File/ directory. Once that’s done, I’ll be able to start working on the IL/ directory. Depending on how extensive the changes are, we may be able to simply delete most of it in favor of the stock version of DevIL, which will make getting a build together much easier, so I’ll keep an eye out while I’m reformatting things.
May take a break soonish. Haven’t really been getting some good entertainment in due to being hyper-focused on MWEdit 😛
Edit:
It’s possible that File/dxffile.h and File/dxffile.cpp can be removed. They don’t do anything but print the data that’s sent to the function and nothing else. The cpp file is actually empty, in fact.
Edit 2:
A bit early but I did some looking and getting a cross-build setup on my end running will take a bit. PopOS doesn’t have all of the packages I’m used to such as the configuration files for various build systems and I’d need to build the library packages manually for Windows so there will be a bit of manual setup (they do have MinGW-W64 stack so that helps). I’ll worry about it when we get to that point, though. VS should still work, though, as the project is already set up for that.
Edit 3:
Okay, finished up reformatting the whitespace in the File/ directory and pushed it and File/XML/ to the repo
Edit 4:
Still looking at the modifications to DevIL and, so far, it looks like it’s simply been changed to force the use of the Windows macros and the DLL support instead of letting that be handled at build time by choosing the options. If that holds, it looks like the modified version can be removed and handled by the stock versions of RevIL or DevIL with the options chosen at build time as designed.
Edit 5:
IL/ is done and pushed. After looking it over, I’m pretty sure the analysis in Edit 4 holds but I’ll take another look when I’m not so tired
In other areas of the code, we have lines like these:
BEGIN_MESSAGE_MAP(CColorStatic, CStatic)
//{{AFX_MSG_MAP(CColorStatic)
ON_WM_PAINT()
//}}AFX_MSG_MAP
END_MESSAGE_MAP()
//{{AFX_INSERT_LOCATION}}
//}}AFX_INSERT_LOCATION
I’m assuming these are generated by VS but I’ve never seen that sort of thing before. If generated, it should probably be implemented in the actual code instead.
WinUtil.cpp/WinUtil.h can probably be trimmed down significantly. They appear to be similar to Common in that they contain a lot of stuff that may or may not be useful but time will tell
The Windows/ directory has had its whitespace reformatted and I just pushed the commit upstream. Just need to do the whitespace in the files in project/ before I can start making other changes. Probably will clean up the excess characters in the return statements and remove the void from parameterless functions
EditViewObj.cpp and EditViewSort.cpp are empty files and can probably be removed. CustRichEdit.cpp has unused if-statements that can be removed to save a few cycles even if they’re probably being optimized out by the compiler
Edit:
Looks like the generated code has something to do with the ClassWizard. Still foreign to me but at least it’s more information than we had previously.
If getting gtkmm built and distributed for Windows becomes overly complicated (it looks a bit daunting at first glance), we could simply wrap the GUI code in conditional compilation blocks to use the native library on Windows and gtkmm on *nix. Won’t be pretty in the code, though.
Still working on the project/ directory. When the newlines were converted to standard \n a few years ago in that directory, a whole bunch of newlines got added for some reason. As a result, I’m adjusting the amount of vertical whitespace meticulously and is taking some time. Fortunately, this should only need to be done once assuming another utility doesn’t barf down the road.
The IsModified() function in the GUI classes can be optmized.
This is an example of what we have currently:
if (m_IDText.GetModify()) {
m_Modified = true;
}
if (m_NameText.GetModify()) {
m_Modified = true;
}
Right now, it checks each condition even though m_Modified is already true so we can change it to else-ifs after the initial if with a default return value of false. That will save a lot of checks when the program goes through those functions
Edit:
There are also blocks like this:
if (pInfoData->Disposition < 0) {
pInfoData->Disposition = 0;
}
if (pInfoData->Disposition > 100) {
pInfoData->Disposition = 100;
}
They can also be optimized into else-ifs. If the first if succeeds, then we already know the second one doesn’t apply.
Side note: still deleting all of the extra newlines.
It appears that GitHub has some sort of CI support for MSBuild. Don’t know if it’s a paid feature or what but I’ll take a closer look at it once I’m done with the whitespace formatting. There’s a chance it won’t work due to the generated code but if we can at least get a Windows build out (there’s currently no build of the program available anywhere except maybe on SourceForge), then that changes the ballgame somewhat and buys us some time with getting the build script together (it’ll be easier to get a build script written once the generated code and DevIL are sorted out).
Edit:
Decided to start looking now. It looks like it’s sort of free: https://docs.github.com/en/billing/managing-billing-for-your-products/managing-billing-for-github-actions/about-billing-for-github-actions
500 MB isn’t a whole lot for regular builds but does give us about 5 – 10 static builds a month, even more for dynamic builds. We won’t be hosting the *nix builds ourselves as library configurations are different for each distro, so more chance of things breaking. For *nix, we’ll simply have the build script and possibly the recipe for package deployment
Here’s some information regarding set up: https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners
Edit 2:
Looks like we can set up custom commands, too. So a build script for the build script? 😛
Will take a closer look later. It uses YAML for the configuration, which I’ve never used before. However, overall, the syntax looks pretty self explanatory. Not sure of the system, though. When I get there, I may start off by looking at the MSBuild template and going from there. Will also need to see how it stores the builds and if we can access them. Time will tell. Still have a while before I can really look at it detail, though. I’m only about halfway through the project directory
There are a variety of cases where OnUpdateItem(esmrecinfo_t *pRecInfo) is an empty function, simply returning 0. Not sure how necessary it is for all uses but I suspect that it can either be removed or moved to the base classes, being overridden as needed.
Buffer.Format(_T("%d"), (int)(byte)pLongData->Strength);
Those lines can probably remove the byte cast as they just cast it to int before the variables are used anyways.
project/EsmRecDialog.cpp and project/EsmRecDialog.h may be broken. It creates GetControlData() but makes it an empty function so we’ll want to test that window to make sure everything works okay before I remove the empty function. From what I can, that class deals with adding a new record so that’s where we’ll want to look. Right now, I can’t tell if it’s an override or the base class as I haven’t found a good class diagram generator that includes a bunch of stuff we don’t need so we’ll need to go through the inheritance chain by hand to check.
It should be easier to sift through things once I fix up the includes. As it is, MWEdit doesn’t include the files it needs explicitly in each file, relying on the includes from other files it does include and some VS assumptions, so fixing that up will help tremendously with sorting out what’s what.
The script compiler has some form of support of an old version of MWSE. It’s not enabled by default and must be turned on at compile time. It may be worth having the MWSE devs take a look at those bits of code and seeing what kind of state it’s in.
l_EsmScrCharTypes could use more comments for its members. Right now, only a handful are commented so we’ve got long strings of macros and 0s that are just sitting there, purpose unknown
Throughout the code, the program uses a mixture of custom string types and standard string literals. Ideally, these would be reduced to the types just in the standard but we’ll need to investigate further. The custom string types are a mixture of a character array TCHAR * (not sure what TCHAR stands for) and its conversion function looks to be _T() and there’s another type called CString(). Then it also uses standard type string literals but not standard c-strings or C++ string types.
Edit:
Finished reformatting the whitespace in the script compiler. I’ve gone ahead and committed and pushed up the changes to that file separately as the file was 5500 lines to start with and I didn’t want to risk losing those changes. Will get the rest of the project/ directory up with the rest of the files.
I’ve spotted some stray symbols here and there so there may be some syntax errors that need to be resolved when we run things through a compiler. We’ll find out!
project/EsmScriptFuncs.cpp defines short versions of the macros for the script functions and you can’t tell what the names mean at a glance. For an example, it defines 0 as VN and ESMSCR_FUNC_BYTE as VB. These should probably be changed to use the long form
Edit:
Looks like I should be done with the bulk of the whitespace reformatting sometime next week or the following week. I know for a fact I missed a few bits here and there but I’ll get those as I go through and do the other cleanup tasks.
It’s a pain, CString is an object, TCHAR is a typdef macro that determines char_t types on MBCS (1 or 2 byte characters) or unicode build. CString should be easier to work with.
Sounds good. 🙂
In an ideal world, I’d like for it to use std::string for string types and char * (or whatever the current standard way of doing it is; I’ve even seen int * used) for raw data blocks. If CString implements more than std::string then we may be able to simply create a subclass and go from there. Will add it to the list!
Still reformatting. Been on the script functions file for a few days now. It’s another one with a lot of arrays. It started out less than a thousand lines of code and is now hitting 6,000 😛
At some point, it may be a good idea to move some of these to-list items to the issue tracker so we can better keep up with them
