MWEdit

[Spastic Hamburger]

Got another one to be added to the issues list:

Function = CellUpdate
	Options = 0x2000
	Opcode = 0x1013
	Return = 0x2, "None"
	Desc = "Broken. Updates the current objects cell position."
End

It’s possible this is related to issue 4. Unless, that is, it means the game itself. The Face function has something similar so I’m leaning towards the latter. In which case, the functions that are mentioned as broken need to be checked in OpenMW and a note added if OpenMW fixes them.

This file is a long one and needs to have its extra newlines removed. Lot of pressing the DEL key 😛

[DeVaultSetter]

“Broken” inspires only a piddle of confidence, might be worthwhile adding a ref. either to the issue or as a new issue with a “related” tag.
Back to the macros:

/* Shorter definitions for convienence */

It’s more of a conveyance of long to short if you will, else the main issue being the byte size of the cpp to increase dramatically when implemented.
Are tabs to be used in the #define sections? Right now it looks a little higgledy-piggledy in EsmScriptDefs.h.

[Spastic Hamburger]

Yeah, we’ll definitely want to expand on the function descriptions to explain why something is broken.

Elsewhere, a compiled help file is referenced but there doesn’t seem to exist any file that sets one up in the repository so more digging is probably needed. It may be using the one in the original CS, which would be strange as MWEdit is a replacement

CHM files were deprecated a while back, weren’t they? Did Microsoft issue a replacement or do we need to look at third-party libraries for a built-in help system?

Yeah, we’ll need to see how things change with the macros. First order of business is to change them all to static constexpr types for modernization and optimization reasons. That file in particular is quite long so replacing the somewhat meaningless abbreviations may be painful (it’s about 8500 lines after the reformat) anyways.

Tabs are used only to indent blocks while spaces are being used for alignment. Don’t worry, all macro sections are being lined up with additional spaces to make them look nice and pretty 🙂

[Spastic Hamburger]

On a similar note, the descriptions for the resistance functions just say things along the lines of, “Get resistance”. They should probably include the stat that they are resisting in the description for completeness

Side note: this is going to be a huge commit when I’m done with the whitespace in this directory. 😛

[DeVaultSetter]

The #defines in project/EsmScriptFuncs.cpp could also remain short with the long defs as a comment. Although they do seem to be quite at home in EsmScriptDefs.h, in case of being used elsewhere. Using static constexpr might be a thing, macros are cheaper in the preprocessor – there may only be a minimal difference.
Ok, as you know, horizontal alignment with tabs can be iffy in any context, as most editors (including Github) have width settings. And don’t start on fonts and their custom scaling. 😛

[Spastic Hamburger]

Yeah, comments may help. I’ll definitely want a good review once I get the whitespace pushed up in a few days. 🙂

Astyle took care of the mixture of tabs and spaces for indentation a few months ago but I always turn on whitespace viewing in my editor to make sure nothing creeps in. I like to use tabs for indentation as everyone has different preferences for tab size. 🙂

Yeah, fonts can be annoying. That’s why I always use a monospace font 😛

In InitInstance() in project/MWEdit.cpp we have CString path = "E:\\GitHub\\MWEdit\\project\\Debug\\MWEdit.ini";. That needs to be changed. Most people only have a C:\ partition and sometimes D:\. It isn’t in a conditional so there’s a good chance the program crashes when it hits that line as the file can’t be created or accessed. Not even sure where that INI file comes from. I don’t recall seeing anything about one, just the registry where settings are stored, but I may have missed it. Loading from the registry in that function is also commented out. Just did a search and that’s the only line where that file is mentioned:

grep --recursive -n "MWEdit.ini" .
./project/MWEdit.cpp:358:	CString path = "E:\\GitHub\\MWEdit\\project\\Debug\\MWEdit.ini";

The next line says it changes the name of the file before it’s used so the file path may not be an issue. I’m still not sure it uses an INI file but I’ll go back and check after I look at setting up build automation. I’m also still concerned about it trying to read from the registry without initializing the registry path first but I don’t know anything about working with the Windows Registry through code, just as a user.

A few lines later, it tries to read from the registry even though the loading from registry code is commented out. This will take investigation for sure so we probably want to get it up on the issue tracker

May be fixable by simply uncommenting the registry lines and commenting that line.

For reference, here are the lines that concern me:

/* Change the registry key under which our settings are stored. */
//SetRegistryKey(_T("MWEdit"));
//FindMWRegistryPath();

//Create the .ini file in application's folder
CString path = "E:\\GitHub\\MWEdit\\project\\Debug\\MWEdit.ini";
//First free the string allocated by MFC at CWinApp startup
//The string is allocated before InitInstance is called..
free((void *)m_pszProfileName);
//Change the name of the .INI file.
//The CWinApp destructor will free the memory.
m_pszProfileName = _tcsdup(_T(path));
/* Load standard INI file options (including MRU) */
LoadStdProfileSettings(0);
/* Load the options from the registry */
m_Options.ReadFromRegistry();

Edit:

Looks like some of the fonts are hardcoded to use MS Sans Serif. We’ll probably want to change them down the road to use more generic font aliases to allow more font flexibility

[DeVaultSetter]

Might be best to comment out the ini file references as the idea was never carried through. Settings seem to be all stored through registry then, which implies additional keys are to be included at uninstallation. Not sure why those registry functions in the above snippet are commented out other than for WIP reasons.
Diagrammatical map of what should go in and out of registry may also help.

[Spastic Hamburger]

Yeah, that’s my thinking too. We’ll probably want to test things to see what happens with the current state first. Ultimately, the plan is to replace the registry settings with an INI file but that’s a bit further down the list.

Got a big surprise: the whitespace reformatting is done! And pushed up! For a general idea of how large the current commit is, the total number of changes in project/ are around 65000 lines of code 😛

Please take a look!

I’ll need a break but after the review and time to recover from that marathon, I plan to work on some admin stuff such as trying to configure Windows build automation to help streamline things 🙂

[Spastic Hamburger]

Looks like project/ColorStatic.cpp had some special characters I didn’t see. I’ll fix that soon.

Yuck, it really got garbled. No idea how that happened. It was fine a few weeks back. Guess I’ll fix it today and take my break later

Edit

project/EditViewObj.cpp, too. Looks like it has to do with the characters that were there. Please list the files that are garbled and I’ll work on them!

Yeah, the line endings is the issue. Some of them were set to Mac format (carriage return) instead of Windows or Linux format. Weird. Fixing them now. Really wish all three major operating systems would just use the same format….

Edit 2:

Okay, got all of the line-endings fixed. May want to set a repo level policy at some point but most editors handle things better than they did twenty years ago. Ready for a review!

And a question: I also set all files to UTF-8. Some of them were set to with BOM while all but a few didn’t have the BOM. Is it safe to remove the BOM from the other files? I’m pretty sure it is but want to verify before I go through and sync everything up

[Spastic Hamburger]

So I got bored and started looking at build automation. Got the basic action script committed and it builds successfully but I’m not sure how to get the build file out of there. GitHub’s help isn’t exactly…helpful in that regard. It says there should be an “Artifact” section here but one doesn’t exist. There’s another help file that says it needs to be added to the repo itself but I’m reluctant to use that method to store binaries in the repository as that would cause a lot of unneeded bloat. The DLLs from DevIL are bad enough

I’ve never really used the feature so it may not be possible to download the result

Edit:

The second article is correct but uses a bad example. Found a better one: https://www.geeksforgeeks.org/devops/download-github-actions-artifacts/

Will take a closer look later but looks pretty simple to set up. Really should be in the template GitHub provides

[DeVaultSetter]

The BOM can be removed so long as the files don’t contain weird unicode characters (not of 1252-like), code editors should be fine without them. Less of a drama in ‘Nix apparently.
Sounds complicated, as haven’t wandered down that particular path.
Just a h/u, two branches of MWEdit are being updated, think you want the Walrus one as current. 🙂

[Spastic Hamburger]

Great, thanks. Will get the BOM removed soon. No special characters are in here, just ASCII. There were a couple weird characters that slipped in functions.dat instead of what should have been an apostrophe (the rest were properly apostrophes) but I’ve already fixed that 🙂 It was probably due to some old artifact years ago that was never caught. There’s a chance it may have fixed a bug or two.

Yeah, GitHub Actions isn’t at all well documented, save for the basic syntax, so I’m mostly stumbling through it. Still need to find out where the actions themselves are documented and why we need to specify the version. I have an idea for automated releases that may be worth exploring once I get the basics set up.

Yep, Walrus is going to be the primary. The one on my profile is a secondary one for when I don’t want to push to the org right away. The one on UESP is Dave’s old one that he mirrored from the last version of the one on Sourceforge that’s just the shell. Once I finish getting the basic build automation set up to let us download the builds, I’ll be making the dev branch the unstable branch and setting master up as the stable branch to help conserve build resources. 🙂

[Spastic Hamburger]

We have a downloadable build! If it runs, I can go ahead and get it added to the Releases page manually as you need to be logged in to GitHub to download build artifacts. Even with the limitation, it’s huge progress! I’ll also need to make an official announcement on the various sites since it’s the first build that’s been available in twenty years 🙂

It looks a bit small so there’s a chance the assets aren’t being packaged up as defined in the RC files. We may need to tweak the packaging script in the YAML file. Or simply distribute the assets alongside. Not entirely sure how to go about it but let’s see if the executable works first 🙂

I also found the list of actions that GitHub supports: https://github.com/actions

Looks like my dream of automated releases is dead: the two actions for releases are listed as unmaintained so that won’t do. We may just need to package things up manually periodically. A bit annoying but we’ll manage 🙂

Right now, the build artifact is a dynamic build so you’ll need to make sure the DevIL DLLs are alongside the executable when you run it. When I package it up for release, I’ll probably simply copy them over to the archive. Not a good long-term solution so we’ll need a better one. We could take a simply sledgehammer to it and make it static, which I’ve done in the past, or we could set it up as semi-dynamic. GCC supports two options, as an example: -static, which is a full static build with everything linked together in one executable, and -static-libstdc++, which is partially dynamic and only statically links the C++ library to it. Not sure of the various options in VS.

I’m also not sure if we can copy over the DLLs in the YAML file. It’d be nice to fully automate things, though. Will do some more reading at some point.

That being said, I don’t really know what the best way of handling the release packaging long-term would be. The options I’ve listed aren’t exactly all that elegant.

Edit:

Looks like something removed the BOM from the few files that had it automatically while I was doing repo work yesterday. Will need to take a closer look but I didn’t see it in any of the files today.

[Spastic Hamburger]

Okay, the program runs successfully and, according to the log file, we’ll also need to package up the .dat files to go along with things. There’s also another file that’s referenced in the code but doesn’t exist in the repo. Here’s the log:

02:55:00 (12) - Error (9 / 2): No such file or directory
02:55:00 (12) - 		 User Message: Failed to open the file 'Functions.dat' (mode was 'reading')!
02:55:00 (12) - Error (9 / 2): No such file or directory
02:55:00 (12) - 		 User Message: Failed to open the file 'customfunctions.dat' (mode was 'reading')!
02:55:00 (15) - Error (9 / 2): No such file or directory
02:55:00 (15) - 		 User Message: Failed to open the file 'mweditextrafile.esp' (mode was 'reading')!

Any idea what mweditextrafile.esp is? The options dialog box says it’s for extra script records but isn’t distributed in the repository.

I’ll go ahead and reach out to Dave and ask. It’s not in his reuploaded repos either.

Once we figure that bit out, I can go ahead and bundle everything up for a release.

[DeVaultSetter]

Great! 🙂
Ah, so the Walrus-Tech is the dev and the DS one the stable?
Extra scripts are explained in features.txt, with a button for it in the gui. 🙂

[Spastic Hamburger]

Got the files from Dave! He found them in an archive he had and added them to his MWEdit repo so we’ll be good to go just as soon as I compile the change log from rfuzzo’s and NimrodPSI’s forks 🙂

Walrus Tech is the organization one and should be thought of as the main one for both stable and unstable. Mine will be for things that change a lot and should probably be reviewed first through a pull request before adding upstream to keep things fair if we wind up with more contributors 🙂

Unfortunately, we can’t open a pull request to contribute directly back to the UESP repository on account of the licensing incompatibilities and the different directory structure. The latter would introduce a lot of merge conflicts but I can try to fix the former. I’ll get a new issue opened soon pinging them to see if I can get permission to add the MIT license to ours to help with everything.

Edit:

For reference, the files we need to bundle for a release are MWEdit.exe, customfunctions.dat, DevIL.dll, Functions.dat, ilu.dll, ilu-l.dll, ILUT.dll, mweditextrafile.esp, and MwEditScriptTest.esp

Edit 2:

We’ll want to review this commit. That’s where our missing file went. There may be other files in there that need to be restored as well. I’ll create a new issue for it

[Spastic Hamburger]

We have a release! Version 0.6.3 is out 🙂

Will get it posted on various sites and we may want to create a blog post about it at some point 🙂

Edit:

Added some additional to-do items to the issue tracker. I’ll add more as I think of them. Feel free to do the same!

Edit 2:

Got the release announcement posted to AFK, Reddit, and OpenMW. UESP will need to wait as I’m blocked from posting external links. I’ll need to ask someone over there about that.

Any other sites that need the announcement?

[DeVaultSetter]

Congrats! 🙂
Robin at UESP would be not at all troubled to help with that. Links to the release sites will want to be somewhere prominent as well. Why not release it at NexusMods, it’s kiddie safe no?

[Spastic Hamburger]

Sounds good. Will add them to the blog post when I get to crafting it 🙂

So far, we have:
Reddit
OpenMW
AFK Mods
Nexus Mods

Can certainly make the same post on Nexus, didn’t even think about it. Will get it soon!

Looks like Robin hasn’t logged in to the UESP forums since 2021 but I can try pinging some of them in a few days. They want you to make a post in the intro thread first if you want to get approved earlier so I’ll give them some time to see mine. 🙂

Edit:

Didn’t happen on my test system the other day but we’re getting reports from some users that Windows Defender is incorrectly flagging the release. Going to need to look into that at some point

Edit 2:

okay, got a thread up on Nexus

[Spastic Hamburger]

Okay, I’ve opened up a new issue to see about getting permission to add a license 🙂

I’ve also opened up several more issues for things that need doing.

I think I can get back to the code now. Not entirely sure….

[Author]

Posts