Adds SAVE_TYPE_ERROR_SCREEN

[pkmnsnfrn]

Description

• Adds SAVE_TYPE_ERROR_SCREEN. If enabled, this shows an error message when the game is loaded on a cart without a flash chip or on an emulator with the wrong save type setting instead of crashing.

Details

Usage

SAVE_TYPE_ERROR_SCREEN

Developers must change this value to TRUE in include/config/save.h to enable.

Given Text

"{COLOR RED}ERROR! {COLOR DARK_GRAY}Flash memory not detected!\n\nIf playing on an emulator, set your\nsave type setting to\nFlash 1Mb/128K and reload the rom.\n\nIf playing on hardware, your cart\ndoes not have a working flash chip."

Testing

Clean Branch

You can recreate this branch by applying a patch or pulling the repo. From a clean version of expansion's upcoming, you can either:

Patch

wget https://files.catbox.moe/4bsipp.patch -O save.patch ; git apply save.patch ; rm save.patch

Repo

git remote add psf-expansion https://github.com/PokemonSanFran/pokeemerald-expansion/ ; git pull psf-expansion saveTypeError

Manual Tests

After replicating the branch, to recreate my testing environment, you can either directly download the debug script, or manually create the changes.

Download

SAVE_TYPE_ERROR_SCREEN == TRUE

wget https://files.catbox.moe/32u54c.h -O include/config/save.h

SAVE_TYPE_ERROR_SCREEN == FALSE

wget https://files.catbox.moe/ier3wd.h -O include/config/save.h

Manual Testing

• Modify include/config/save.h to the desired values
• Compile
• Change emulator setting AWAY from 64K
• Load game

VisualBoyAdvance

I can't figure out how to change the save type in mGBA 0.10.2, so I'm using VisualBoyAdvance-M 2.1.9.

To change the save type: Options > Game Boy Advance > Configure > Save Type

Verified Scenarios

All videos show the steps described in "Manual Testing"

TRUE

Zr4FbwX.1.mp4

FALSE

QbH4p3z.1.mp4

People who collaborated with me in this PR

This was originally written by by Anon822. Give them all the credit.

Discord Contact Info

I am pkmnsnfrn on Discord.

[mrgriffin]

I think this PR needs to have the compound string indented by one "block", but since it's inside another function maybe it needs to be indented by two blocks so the last line of the function can still be indented by one?

I will wait until MGriffin weighs in then. I don't have a strong opinion, but please respond on the PR once a decision is made

Oh I see, hmm... Yeah I suppose you'd expect to indent that like:

    SaveFailedScreenTextPrint(
        COMPOUND_STRING(
            "{COLOR RED}ERROR! {COLOR DARK_GRAY}Flash memory not detected!\n\n"
            "If playing on an emulator, set your\n"
            "save type setting to\n"
            "Flash 1Mb/128K and reload the ROM.\n"
            "\n"
            "If playing on hardware, your cart\n"
            "does not have a working flash chip."),
        1, 0);

Since this PR is going to upcoming an alternative that could be considered is:

    static const saveFailedMessage[] =
        "{COLOR RED}ERROR! {COLOR DARK_GRAY}Flash memory not detected!\n\n"
        "If playing on an emulator, set your\n"
        "save type setting to\n"
        "Flash 1Mb/128K and reload the ROM.\n"
        "\n"
        "If playing on hardware, your cart\n"
        "does not have a working flash chip.";
    SaveFailedScreenTextPrint(saveFailedMessage, 1, 0);

I think either of those is fine. For comparison here's the right-leaning one which I think is probably a bit annoying to read if your editor is narrow like mine... But I imagine it's fine for most people?

    SaveFailedScreenTextPrint(
        COMPOUND_STRING("{COLOR RED}ERROR! {COLOR DARK_GRAY}Flash memory not detected!\n\n"
                        "If playing on an emulator, set your\n"
                        "save type setting to\n"
                        "Flash 1Mb/128K and reload the ROM.\n"
                        "\n"
                        "If playing on hardware, your cart\n"
                        "does not have a working flash chip."),
        1, 0);

To be honest, I don't have strong opinions about any of the choices. Weakly I'd order them 2, 1, 3. In this example I think 1 and 2 are pretty interchangeable, but if there were a lot of arguments to the function, or the string wasn't the first or last argument, I could see 2 making it easier to follow what's happening.

even then we need to agree upon how to break \n\n

I tried with \n\n together and broken, and personally I think it's more clear how the message will render with them broken.

[AlexOn1ine]

In this example I think 1 and 2 are pretty interchangeable, but if there were a lot of arguments to the function, or the string wasn't the first or last argument, I could see 2 making it easier to follow what's happening.

I agree with this. I would say generally compound strings in function calls looks a bit weird.

[pkmnsnfrn]

I tried option 2 and either I did something wrong or it doesn't work: https://files.catbox.moe/7ou4w3.c
so 7953c34 is option 1

[mrgriffin]

I tried option 2 and either I did something wrong or it doesn't work: https://files.catbox.moe/7ou4w3.c so 7953c34 is option 1

Whoops, my bad! I think the problem is that I forgot the _() around the string 🤦

-static const u8 saveFailedMessage[] = //prints garbage
+static const u8 saveFailedMessage[] = _(
     "{COLOR RED}ERROR! {COLOR DARK_GRAY}Flash memory not detected!\n\n"
     "If playing on an emulator, set your\n"
     "save type setting to\n"
     "Flash 1Mb/128K and reload the ROM.\n"
     "\n"
     "If playing on hardware, your cart\n"
-    "does not have a working flash chip.";
+    "does not have a working flash chip.");