Remove CompatTool add from setflatpak function

[HanPrower]

It isn't necessary due to how the Flatpak STL works, and at this point in the setup $SROOT doesn't seem to have been set anyway, so it fails.

Wed Aug 28 15:27:44 BST 2024 INFO - setflatpak - seems like flatpak is used, because variable 'FLATPAK_ID' exists and points to 'com.valvesoftware.Steam'
Wed Aug 28 15:27:44 BST 2024 SKIP - CompatTool - Steam Home Dir '' not found!

[HanPrower]

Whoops, forgot to make a branch and now posted 2 commits.

Well, the second one is a fix for gamescope -W/-H. Wrong case in the grep, so if it wasn't set the dropdown would end up as just an "x" instead of a resolution.

[sonic2kk]

Thanks!

The Flatpak changes make sense.

The GameScope changes don't look correct, we check for -W and -H below that: https://github.com/sonic2kk/steamtinkerlaunch/pull/1152/files#diff-77c6d4bb1bf7c1988ff5a068c856b6c24e113663f2e61cb3a2c98723c0fafc04L11321

Is the check for -W and -H below not working?

This is why looking at a diff on a phone screen is a bad idea :-) I see now. Our check for if the line exists in the arguments is out, we're looking for lowercase -w and -h when we should be looking for uppercase. Sorry about that, yeah, makes sense now!

[HanPrower]

The first one is meant to be detecting upper case W and H and the second one below is meant to be detecting lower case w and h.

Currently both lines are identical.

The top one is in the wrong case and the commit fixes that.

[sonic2kk]

Updated my comment, sorry for the sillness 👍 This GameScope fix will allow us to properly parse the -W and -H flags from the GameScope arguments. Since it's a ! check we will fall back to 1280x720 now only if -w and -h aren't present (i.e. weren't defined in the GameScope argument list). I guess this has been broken for a long time and no one reported it, that's crazy. We've been checking it wrong for 18 months according to the blame. I guess no one has left "Game internal resolution" blank before 😅

ShellCheck is failing because FPBIN is not used anymore. I guess it was used before to set the path when using CompatTool but nowhere else. The second argument to CompatTool is the path to the real script so that we can create the compat tool prefix.

If you can remove it (or comment it out, if you think it could be used in future), and bump the version (PROGVERS) to something like v14.0.20240828-1, I think this is good to be merged.

[sonic2kk]

Hmm, testing the change locally, it doesn't seem to fix the issue. Leaving the "Game internal resolution" blank still results in the dropdown showing x. In fact, this also happens with "Game show resolution".

Not sure what's up with that yet, but your commit doesn't make the issue worse at least so we can leave it in and I can dig further into it separately to this PR.

[HanPrower]

Oh, when I tested it it worked... then again I only left one blank x.x

[sonic2kk]

Thanks! I have merged the changes 😄

Changes look good overall though, and thank you for bringing this issue to my attention! If it's something you want to continue looking at please feel free to, otherwise I can dig into it (I have some other GameScope flags I want to add soon).

I really appreciate the work here to investigate issues, clarify the Flatpak + GameScope issue, and to notify me of the PID issue.

[sonic2kk]

Okay, so the GameScope problem is a little tricky, but I think I have a solution.

The problem is that we currently only check if the -w and -h (or -W and -H`) flags are set and we incorrectly assume that if the flags are present, corresponding values will be as well.

We check these as a pair to make sure they exist, so that if one is missing, we invalidate the other, so that we don't end up setting the width but not the height. We could theoretically change this, GameScope itself will accept simply -h and calculate the width based on a 16:9 aspect ratio, so we could do that on our end, but that's a separate thing.

Basically, the current check just makes sure the width and height flags are set, and if they aren't, we default to 1280x720 (matching GameScope behaviour).

But if on the UI we make these fields blank, we still save the -w -h -W -H flags. This means our ! grep -qw "\-W" <<< "$GAMESCOPE_ARGS" || ! grep -qw "\-H" <<< "$GAMESCOPE_ARGS" check will actually pass, because the flags are set. That results in our check to parse out the width and height based on this to return garbage. It actually returns -hx-h or -Hx-H, but we end up parsing this out on the UI using some logic to separate out the width and height and using regex to only get numbers before and after the x ($(printf "%s\n" "$("$XRANDR" --current | grep "[0-9]x" | awk '{print $1}' | grep "^[0-9]" | tr '\n' '!')")")), and since there are no numbers, we end up with nothing to display.

The core problem lies with "$(tr ' ' '\n' <<< "$GAMESCOPE_ARGS" | grep -wA1 "\-w" | tail -n1)x$(tr ' ' '\n' <<< "$GAMESCOPE_ARGS" | grep -wA1 "\-h" | tail -n1)". We can break this down a bit to illustrate.

# This will return `-h`
GAMESCOPE_ARGS="-w -h"
tr ' ' '\n' <<< "$GAMESCOPE_ARGS" | grep -wA1 "\-w" | tail -n1)

The reason this returns -h is because:

• tr ' ' '\n' <<< "$GAMESCOPE_ARGS" separatees each part of the GameScope args list onto a newline delimited by space. So we end up with -w and -h on newlines.
• Then we try to grep for -w and the line after it, which should be the value given to the flag. This is where the incorrect assumption comes in: This logic assumes that the next line after -w will be its value, because we assume if we have the flag, we have the value, but this is not a correct assumption. So we end up getting -h, the item on the next list. tail -n1 gets us the last line from the output, which is where we end up with -h.

The fix is to check if we have a value for -W/-w and -H/-h and if we don't have one for either, to return 1280x720. I'll have to think about a clean solution.

Something like the following does work, but it's a bit ugly.

# GameScope Show Resolution (corresponds to -W, -H options, uppercase) -- Actual GameScope window size -- Dropdown
if [ -z "$GSSHWRES" ]; then
	if ! grep -qw "\-W" <<< "$GAMESCOPE_ARGS" || ! grep -qw "\-H" <<< "$GAMESCOPE_ARGS"; then
		GSSHWRES="1280x720"
	else
		# TODO these should be 
		GSARGSHOWWIDTH="$( tr ' ' '\n' <<< "$GAMESCOPE_ARGS" | grep -wA1 "\-W" | tail -n1 | grep "^[0-9]\+$" )"
		GSARGSHOWHEIGHT="$(tr ' ' '\n' <<< "$GAMESCOPE_ARGS" | grep -wA1 "\-H" | tail -n1 | grep "^[0-9]\+$" )"

		if [ -z "${GSARGSHOWWIDTH}" ] || [ "${GSARGSHOWHEIGHT}" ]; then
			GSSHWRES="1280x720"
		else
			GSSHWRES="${GSARGSHOWWIDTH}x${GSARGSHOWHEIGHT}"
		fi
	fi
fi

We could clean it up a bit and just make this a function and re-use it for GSINTRES. We probably still need to check if the resolution flags are passed and default if they are not as well as checking if the flags are present but are not given a value. Or maybe there's opportunity to use getGameScopeArg here to check for -W, -H, -w, -h, since it's designed for this purpose, and then if we have a value for both we can combine them into a string, and if one of the values is blank, default to 1280x720. I'm not sure how feasible this is, I remember running into trouble trying to use getGameScopeArg for this when it was implemented, but I could try again :-)

EDIT: It seems like we can use getGameScopeArg but it ends up with the same problem :-)

[sonic2kk]

Came up with a potential fix for getGameScopeArg to ignore non-number (integer or decimal) values that it parses out when it checks for number values. I refactored getGameScopeArg a bit in the process too. I'll need to test quite a bit, but basically we can use grep -P "^([\d]+)(?:\.([\d]{1,2}?))?$" to extract only numbers from the tr ' ' '\n' <<< "$ARGS" | grep -wA1 "$FLAG" | tail -n1 check for flags where we expect numerical values. This means if we parse out -h from -w -h, we will default back to 1280x720.

The resolution boxes are special cases, because they are comboboxes that represent two GameScope flags. Most Yad NUM elements have logic to make sure they can't be blank and get a sensible value, but because the resolution boxes are actually just combobox entry fields, they could take any value because they are free text.

So the refactor changes shouldn't affect other numeric values, but I will need to test to make sure it doesn't impact other comboboxes just to be safe. I'll also probably do a full pass on the GameScope flow to make sure nothing has broken, as getGameScopeArg is a pretty integral part of the whole GameScope flow.

[sonic2kk]

PR to fix the GameScope resolution boxes returning 'x' is up at #1154.

[sonic2kk]

Forgot to mention, I gave you credit for this fix on the changelog under "Fixes" :-) https://github.com/sonic2kk/steamtinkerlaunch/wiki/Changelog