A fix for the broken version checker

[awsdert]

I've had instances where I've had an experimental version of a mod installed (which was greater than the live version) and the mod manager DOWNGRADED my mod when I hit update/update all, some of them were simply deleted altogether. I expect that is because of how the version comparison was overthinking the checks, if the text in part of the version is not a number then it should be treated as 0, that has been the case in most software since someone introduced atoi (I think it was microsoft), I believe even the scanf family of functions do likewise so there's no reason the mod manager should not also do likewise. Here's my proposed fix for the version check.

The async issue should be resolvable by the mod manager opening up a thread dedicated to downloading and installing stuff, it should use a folder based queue, this is so that the download can be paused until the game is rebooted if for some reason the game is closed before it done. The dedicated thread is so that changing UI focus does not effect the download & installation. The deletion issue would be resolved by simply waiting until the download has finished before deleting it and renaming the downloaded folder to the deleted folder.

        public static ulong VersionToInteger( string version )
        {
            if ( version != null )
            {
                int i = 0;
                ulong ver = 0;
                ulong const mul = 1000;
                string[] parts = version.Split('.');
                for ( string part in parts )
                {
                    ++i;
                    ver *= mul;
                    if ( uint.TryParse(part,out uint val) )
                        ver += val;
                }
                for ( ; i < 3; ++i )
                    ver *= mul;
                return ver;
            }
            return 0;
        }
        public static bool IsVersionHigher(string live_txt, string held_txt)
        {
            ulong live_ver = VersionToInteger( live_txt );
            ulong held_ver = VersionToInteger( held_txt );
            return (live_ver > held_ver);
        }

[KYPremco]

Semi fixed, it will be done in upload order now. Mod.io versions are strings, and not everyone is using a semantic versioning system. Specially not in maps.

[awsdert]

I'm confused, what part of string version made you think I wasn't starting with a string???

[KYPremco]

The part that it won't always (specially map makers) used that way. For example: V-5 (for Patch 5) Or: '' <-- nothing at all.

With the solution you provided, it's assumed to have a fixed layout. Which mod.io does not provide.

[KYPremco]

So I decided that basing it on upload time would be the most suited, as mod.io does this theirself as well.

[awsdert]

(for Patch 5)

Uh, but that still equates to 5 right? Aside from needing to check for the - in addition to the . that doesn't really change anything. It's still starting with a string.

[KYPremco]

First release, second release. Does not have an number. Working update 5, Working update 5 patched. Both have 5.

But for how it won't be implemented. Maybe another time something similiar will, but at this point of time I don't think it's better.

[awsdert]

Well it's better than what I found, been too many months so I don't remember the location but you did have a version string converter somewhere which overcomplicated the string to integer conversion. Pretty sure it was named IsVersionHigher too which is probably why I included that at the bottom with the cleaner method of conversion to integer moved out of it into it's own function (VersionToInteger). THAT's what this was for, whether you use upload dates to differentiate uploads with matching versions is out of scope and not relavant to what I was fixing.

Edit: You could easily add that to the bottom of the IsVersionhigher I provided, just make sure to separate the code that identifies the dates into it's own function to help with readability should you ever come back to the function again.

Edit 2: While you could add it to the bottom I think it's better to leave the function as is and instead use a wrapper function that tries by version 1st then upload date, in which case it would be best to change the bool to long and return the result of subtracting the held_ver from live_ver. In the event 0 is returned you check the upload date. In the event -N is returned you know the download was older than the existing install. In the event +N is returned then you skip checking the upload date since you already know it newer