Fix-gitignore-autogenpath

[WisoAltred]

Fixes the path to be the proper path for gitignore. From #1149.

[stonedDiscord]

maybe add both

[WisoAltred]

Why? The other path doesn't lead into anything?

[stonedDiscord]

In your machine it doesn't but on Linux the app is named AttorneyOnline without the underscore.

[WisoAltred]

Oh, I see. Alright.

[WisoAltred]

So, why does it do that? Why is it not consistent? I mean, there's an underscore after AttorneyOnline_autogen regardless.

[OmniTroid]

the build target is consistently named AttorneyOnline now. only for final distribution are spaces added (Attorney Online.exe, Attorney Online.bin etc)

if you do ./configure clean, ./configure and build it should autogen into the AttorneyOnline_autogen folder correctly

[WisoAltred]

I see. Thank you. This feels like something that should have been noted before any of these changes were put in, both by the maintainer approving and the contributor, but whatever. AttorneyOnline_autogen it is.

That said, what else has been changed? I noticed there's a libqapng.dylib check now, for example?

[OmniTroid]

You're right, the build target renaming should have been displayed better in the PR.

.dylib is the format of dynamic libraries for macOS. it ensures that apng files can be rendered correctly there.

Additionally, the zip files generated from github now has more explicit names, ie.

AttorneyOnline-{platform}-{arch}-{git commit sha}.zip

eg.

AttorneyOnline-windows-x86_64-abababab.zip

[WisoAltred]

Right. I understand that part, I'm more so talking about the changes incurred to the windows qtapng. Are you able to confirm that it properly takes the qtapng.dll from the image formats inside a same-directory qtapng clone?

And, I see, that's an acceptable change, thank you.

This is somewhat hijacking of this PR to address a prior PR, which isn't that good, so I apologize for that. I think that can continue for now regardless.

[OmniTroid]

Yes, I tested it on Windows and the app is configured to complain very loudly if apng support is missing. (info box)

You can check this by downloading the latest Windows artifact built from master: https://github.com/AttorneyOnline/AO2-Client/actions/runs/32830280648

[OmniTroid]

One more thing.. the (primary, that is the one written to) base folder was added to About:

Because it's a very, very important folder and the resolution logic across OSes can be a bit finicky, so it's the kind of thing that's nice to have transparent.

[WisoAltred]

Nice. Alright. And yeah, showing the base folder isn't a bad idea, especially with mounted folders being a possibility, though it does show there. Either way, there is no harm in it.

Could I ask you to provide the source of your qtapng (windows)? I assume you're building it with MinGW, or llvm-mingw.

[OmniTroid]

It's all handled in configure.sh, on line 332:

AO2-Client/configure.sh

Line 332 in 96d9f3e

get_qtapng() {

It simply pulls the source code from another git repo (https://github.com/jurplel/QtApng.git) and compiles it per platform. The whole bash function looks like this:

get_qtapng() {
    echo "Checking for Qt apng plugin..."
    apng_build_dir="./qtapng/plugins/imageformats"
    imageformats_dir="./bin/imageformats"
    APNG_LIB=""
    if [[ "$PLATFORM" == "windows" ]]; then
        APNG_LIB="qapng.dll"
    elif [[ "$PLATFORM" == "linux" ]]; then
        APNG_LIB="libqapng.so"
    elif [[ "$PLATFORM" == "macos" ]]; then
        APNG_LIB="libqapng.dylib"
    else
        echo "Unsupported platform: ${PLATFORM}"
        return 1
    fi

    apng_dst_path="${imageformats_dir}/${APNG_LIB}"

    if [ -f "$apng_dst_path" ]; then
        echo "Qt apng plugin is installed."
        return 0
    fi

    if [ ! -d "./qtapng" ]; then
        git clone https://github.com/jurplel/QtApng.git ./qtapng
    fi

    cd ./qtapng

    $CMAKE . \
        -G Ninja \
        -DCMAKE_MAKE_PROGRAM="$NINJA" \
        -DCMAKE_PREFIX_PATH="$QT_ROOT_DIR" \
        -DCMAKE_MODULE_PATH="${SCRIPT_DIR}/cmake" \
        -DCMAKE_C_COMPILER="$CC" \
        -DCMAKE_CXX_COMPILER="$CXX"

    $NINJA

    cd "${SCRIPT_DIR}"

    mkdir -p "$imageformats_dir"

    apng_src_path="${apng_build_dir}/${APNG_LIB}"

    cp "$apng_src_path" "$apng_dst_path"
}

[WisoAltred]

I'm trying it continuously at the moment, it works on MSVC just fine, but on mingw/llvm-mingw it does not recognize it at all. Are you sure you've tried this on both debug/release, on windows, via QT6 (or qt 6.10/6.11) (on any creator version above 19)?

[OmniTroid]

Can you elaborate on "does not recognize it" means? Do you have the full output of your configure.sh?

[WisoAltred]

I think I see the issue.

I do indeed have QT ofcourse, but its not where I can imagine its attempting to look for it in.
I'll set the $QT_ROOT_DIR manually and see what happens.

[WisoAltred]

May I ask what the difference is between QT_ROOT and QT_ROOT_DIR?

[OmniTroid]

QT_ROOT is C:/Qt while QT_ROOT_DIR is C:/Qt/6.5.3/mingw64 (full path to the installation)

QT_ROOT is only used internally while QT_ROOT_DIR is what's passed to configure.sh (matching CI behavior)

[WisoAltred]

Right. This is somewhat unfortunate to do everytime for users who do not have qt on C:, which to be fair is somewhat rare, but they have to change all of these and account that their QT_ROOT may not necessarily need to be $QT_ROOT_DIR/../..

Either way, it now properly outputs:
Checking for BASS...
BASS is installed.
Checking for BASSOPUS...
BASSOPUS is installed.
Checking for Discord RPC...
Discord RPC is installed.
Checking for Qt apng plugin...
Cloning into './qtapng'...
remote: Enumerating objects: 2278, done.
remote: Counting objects: 100% (353/353), done.
remote: Compressing objects: 100% (141/141), done.
remote: Total 2278 (delta 244), reused 212 (delta 212), pack-reused 1925 (from 1
)
Receiving objects: 100% (2278/2278), 4.18 MiB | 57.00 KiB/s, done.
Resolving deltas: 100% (1112/1112), done.

All of which to say, it still does not work on debug.

I think this is because there is no plugins/image format after the pull, not in the link you've provided for a source, anyway. So I don't understand what exactly its trying to find, but it won't be finding it. Manually putting the qapng.dll in does work, though.

[OmniTroid]

So a couple of things;

Qt Maintenance Tool's default install path is C:/Qt which is where we try to auto-detect if QT_ROOT_DIR is not passed. Qt Maintenance Tool always puts the installation (assuming QT_ROOT here) in C:/Qt/<qt version>/<variant>/. So QT_ROOT is by definition ../../ relative to QT_ROOT_DIR.

Effectively it's as it says in configure.sh help section; either you define QT_ROOT_DIR explicitly or we'll make a guess to where Qt is.

Regarding apng, Your configure.sh output seems truncated and there's no useful information there other than it cloned correctly. Is QtImageformats in the Qt installation?

I'm not very preoccupied on the particular naming. If you think passing QT_PATH is easier we can do that too.

[WisoAltred]

Handled, thank you. And the name is fine.