chore: workflow to verify crowdin contributions

[bahrmichael]

Starting work on a new GHA workflow to verify the integrity of the crowdin files, based on a suggestion from @winex: https://github.com/bahrmichael/factorio-tycoon/commit/2a0ffc3b0559d7b6a70b21c8e99d2d5a55c32625#commitcomment-139268604

The goal is to verify that all keys from the english locale are present in the other languages, that no groupings are missing (e.g. [entity-names] at the beginning of a file, and that there are no duplicate keys that could crash the game on startup).

This is still a work in progress since there are a bunch of edge cases that I didn't have the time to work through yet. The script is mostly generated by ChatGPT and open for discussion and improvement.

[winex]

looks fine, though:

1. i would move that 4x same-regex grep calls into a single bash function (idk, if that's possible for <(), but it should be)
2. $source_file and $target_file inside <() should be quoted with "", unless bash does this internally (i don't think so)
3. variables doesn't need to be quoted inside [[ ... ]] -- that's where bash becomes better than sh (a bit ;)), though it needs that inside[ ... ](condition) or $[ ... ](shell call) to avoid subst exploits, i don't remember which is what, though...

and i would better write like this as per "return/break while error-checking from top-top bottom thing" (idk, how it's called) and keep indentation to a minimum - personal preference, just hate if-then--if-then--if-then--else--end--end--end bloating...

    # Check if the target file exists
    if [[ ! -f $target_file ]]; then
        echo "File $lang/$filename does not exist."
        exit 1
    fi

    # do things after checks from above...
    #...

when code goes out of view to the right, then it's a bad code! :))

[winex]

ooh, great to see that it already does the job (cut from check details):

Comparing locale/en/tips-and-tricks-item-name.cfg with locale/de/tips-and-tricks-item-name.cfg
Files /dev/fd/63 and /dev/fd/62 differ
Differences found!
11c11
< tycoon-passenger-train-stations=
---
> tycoon-passenger-train-station=
Error: Process completed with exit code 1.

that's a sneaky one! ;)

[bahrmichael]

ooh, great to see that it already does the job (cut from check details):

Comparing locale/en/tips-and-tricks-item-name.cfg with locale/de/tips-and-tricks-item-name.cfg
Files /dev/fd/63 and /dev/fd/62 differ
Differences found!
11c11
< tycoon-passenger-train-stations=
---
> tycoon-passenger-train-station=
Error: Process completed with exit code 1.

that's a sneaky one! ;)

I think that one's actually a false alert! The other language files for tips and tricks don't have the multiple. It seems to be highlighting entities within the text. Will tweak it a bit more, but it's good to see that it works in general.

[bahrmichael]

when code goes out of view to the right, then it's a bad code! :))

laughs in ultra-wide :P

[winex]

did some work on this script: winex:crowdin-check

can't you add that commits into this PR as well? split to the death almost every change...

so, it now counts and reports ALL errors, correctly show names (just found --label option) and diff output changed to be:

• -U0 unified with 0 lines output before/after (default is 3 for just -U)
• -N, --new-file treats absent files as empty, which we need

i always use diff -Nurp, but here we don't need *r*ecursive and *p* function hint doesn't work for .ini configs

reports

now it looks nice and clean, see: https://github.com/winex/factorio-tycoon/actions/runs/8190658566/job/22398112575

 1 Run ./compare_locales.sh
 4 --- locale/en/tips-and-tricks-item-name.cfg
 5 +++ locale/de/tips-and-tricks-item-name.cfg
 6 @@ -11 +11 @@
 7 -tycoon-passenger-train-stations=
 8 +tycoon-passenger-train-station=
 9 ERROR: Total errors: 1
10 Error: Process completed with exit code 1.

after removing few files:

ERROR: File does not exist: locale/ru/fluid-name.cfg
ERROR: File does not exist: locale/ru/shortcut.cfg
ERROR: Total errors: 2

when it can't extract from the source file (for any reason, $ chmod a-r ...), error is +1000, like:

ERROR: Unable to extract from: locale/en/controls.cfg
--- cut, see above ---
ERROR: Total errors: 1002

...style

few words about style in my branch - when you use many languages at once, it becomes total hell:

• Lua if true then ... end
• BASH if true; then ... fi
• C-style if (true) { ...; } looks way easier and more common/universal!
• Python is the champ here, smashes all this stuff to just if True: single line

i specifically used function keyword (BASH doesn't require it) for the same purpose here - to be more Lua-like

imho, in shell scripts such if's and things like if [ ]; then $[ ](?) looks better if a script is intended to be old-style/POSIX sh-only, but we use modern shell so it's => [[ $var ]] && { ... } and double-brackets doesn't need var quoting (that it sooo easy to miss!)

though, ShellCheck said replace let VAR+=1 to (( VAR+=1 )) -- this looks soo creepy to me - in what language lines DO start with round brackets/parentheses???!!! (any other one than Lisp, ofc). vars in bash are strings var=0; var+=1 => var == "01", so here is the arithmetic let keyword

[bahrmichael]

I had to make two changes to get the script working on macos (which is the OS that I develop on; I'd be happy to have this script working locally and on GHA). Those are just the hacks I needed to get it to work; there are probably cleaner approaches.

Mainly the --tempdir and -printf don't exist on macos and have to be worked around.

[bahrmichael]

I reverted the macos compatibility so that it can run on GHA. If you're interested in pushing the OS-compatibility further let me know, but until then I don't think that's worth investing a lot of time into (assuming crowdin problems don't happen every other day).

[winex]

sorry, didn't know macos' find has no printf. your change would be the same as printf "%p\n" (lower-case), but i know -print0 is better for weird symbols in paths

will check why it failed on GHA, any points?

i forgot to mention: i suppose only factorio-related files should be in the root. can we move this script into .github/ or some other directory like .scripts/? this is similar to online-graphics/ which is not included in release .zip ...and it's not looking good to have a shell script in the root of a mod!

[bahrmichael]

will check why it failed on GHA, any points?

There's a mistake in my German translation, should be resolved soon.

i forgot to mention: i suppose only factorio-related files should be in the root. can we move this script into .github/ or some other directory like .scripts/?

Done!

[bahrmichael]

It works 🥳 Thank you very much for the help!

[winex]

omg, stat -f on linux is totally wrong option (as you may see in GHA output) - it's:

-f, --file-system
       display file system status instead of file status

correct one: stat -c "%n"

so, can we use uname -s or similar to test for macos? does it output Darwin?

case "$(uname -s)" in
    Darwin*) STAT_OPTS="-f '%N'" ;;
    Linux*)  STAT_OPTS="-c '%n'" ;;
esac

then we'll just | stat $STAT_OPTS

@bahrmichael ok, i'm pushing something into my branch... should work fine on GHA and on macos, i hope... can you check it?

[bahrmichael]

$ uname -s
Darwin

Yup, it yields Darwin.

[winex]

this one: https://github.com/winex/factorio-tycoon/commit/41230ade2a998f119bbd2673567aa985399b948d

should i rebase it onto main and revert a "revert", then apply this fix?

[bahrmichael]

I'll create a new PR, something's not quite working yet on macos. Will provide more details there and link it here.

[bahrmichael]

See https://github.com/bahrmichael/factorio-tycoon/pull/286