LandSandBoat/xiloader

Crash when using `--lang` option

Closed this issue · 10 comments

I've looked at the code and haven't seen any obvious cause but, something about the latest changes makes xiloader totally crash when you use the --lang flag (e.g. --lang US).
Including this flag has been part of Windower's setup instructions so, fixing this would help a lot of people. Thanks.

Edit: I should specify, it crashes after login but before the game client launches.

Did some more testing, seems to work fine standalone, but crashes when passed via Windower. This used to work fine though so, further investigation would be helpful.

Edit: Oh, jk, it crashes like, fairly quickly on launch

Update:
I attached a debugger... this line is spitting out
Duplicate argument

check argv/argc with the debugger, is it actually there twice?

Looks like yeah.
image
I am still finding out if Windower always did that and using --lang {region} never actually did anything, but that's been in our instructions for a long time.

That aside, I suspect that possibly whatever repo/version/etc xiloader used to use as a dependency, possibly defaulted to allowing duplicate optional arguments, and that the new setup defaults to not allowing that.

Interesting. This looks like "not an error" because it's correctly reporting you are doing something wrong, but maybe there's a way to tell argparse to use the last arg in the arguments as the newest one to preserve compatability

If you change this line from

args.add_argument("--lang").help("(optional) The language of your FFXI install: JP/US/EU (0/1/2).");

to
args.add_argument("--lang").help("(optional) The language of your FFXI install: JP/US/EU (0/1/2).").append();
does it work?

Previously we did this all by hand:

        /* Language Argument */
        if (!_strnicmp(argv[x], "--lang", 6))
        {
            std::string language = argv[++x];

            if (!_strnicmp(language.c_str(), "JP", 2) || !_strnicmp(language.c_str(), "0", 1))
                g_Language = xiloader::Language::Japanese;
            if (!_strnicmp(language.c_str(), "US", 2) || !_strnicmp(language.c_str(), "1", 1))
                g_Language = xiloader::Language::English;
            if (!_strnicmp(language.c_str(), "EU", 2) || !_strnicmp(language.c_str(), "2", 1))
                g_Language = xiloader::Language::European;

            continue;
        }

So this would have read duplicate arguments twice, taking the latter.
Should it crash when being fed duplicate arguments? no.
Should your workflow be providing duplicate arguments? also no.

Looks like we've both go some fixin' to do.

I tested before/after with .append() on the arguments.

Before:
(exits)

Duplicate argument
Usage: xiloader [--help] [--version] [--server VAR] [--username VAR] [--password VAR] [--email VAR] [--serverport VAR] [--dataport VAR] [--viewport VAR] [--authport VAR] [--lang VAR] [--hairpin] [--hide]

After:
(works as normal)

I had to sleep but, looks like there has been some incorrect/misleading instructions on our end for years. We definitely should not be passing in the same arg twice, so that is an accident, but the older xiloader definitely accepted the last argument and did not close.
We shall fix our instructions, but this change will help out all the people that followed the misleading instructions for years. Thank you!