Skip to content

eduke32: (QOL) ignore case when check for user game data #3478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 22, 2025

Conversation

s1eve-mcdichae1
Copy link
Contributor

@s1eve-mcdichae1 s1eve-mcdichae1 commented Feb 11, 2022

...so shareware version is not downloaded unnecessarily, in case user has old DOS files in ALL CAPS. Add rename dependency. Modeled after wolf4sdl.

@s1eve-mcdichae1 s1eve-mcdichae1 changed the title Rename game data files using all lowercase eduke32: (QOL) Rename game data files using all lowercase Feb 11, 2022
@s1eve-mcdichae1
Copy link
Contributor Author

(Make $dest dir before pushd so rename is always safe.)

@joolswills
Copy link
Member

Sorry, this reply is a few years late. I think I would prefer to just check for the file in caps as well, rather than renaming user data. I suppose a mixed case filename is possible, but I would think unlikely. Unless there is a specific release with mixed case, which we could handle.

@joolswills
Copy link
Member

joolswills commented May 20, 2025

Thinking about it, the check could be modified to use find "$dest" -maxdepth 1 -iname "duke3d.grp" which would work without renaming perhaps.

@s1eve-mcdichae1
Copy link
Contributor Author

s1eve-mcdichae1 commented May 22, 2025

Am I overthinking or does it need all this:

if [[ -z "$(find "$dest" -maxdepth 1 -iname duke3d.grp 2>/dev/null)" ]]; then

if [[ -z "$(find foo)" ]] because find always return true whether file is there or not, so if ! find will never work the way I think it should? So instead, have to do the find and test for nonzero string length.

Then 2>/dev/null because, if $dest not exist yet, then it return find: ‘foo’: No such file or directory, reads a nonzero string and makes the [[ -z foo ]] test think the file is there and then shouldn't be download?

Edit no this won't fail the download (I guess since it read stdin and not stderr) but still write error to screen (log?) if not suppress.

So, either suppress with 2>/dev/null or else, maybe just put the mkUserDir before and outside the if so that $dest will necessarily always exist?

@joolswills
Copy link
Member

joolswills commented May 22, 2025

You can take advantage of the processing order of conditions:

eg.

if [[ ! -d "/doesnt/exist" || -z $(find /doesnt/exist -iname "somefile") ]]; then
  .. do something
fi

@joolswills
Copy link
Member

Posted on mobile, and had typos - fixed them. But hopefully this gives an idea :-)

@joolswills
Copy link
Member

Making the directory before also seems fine, and makes it simpler.

@s1eve-mcdichae1 s1eve-mcdichae1 changed the title eduke32: (QOL) Rename game data files using all lowercase eduke32: (QOL) ignore case when check for user game data May 22, 2025
@joolswills
Copy link
Member

Changes look good thanks - I just had a thought, technically if someone made a folder called duke3d.grp I think this would fail. I didn't think of it until now, but it's minor and unlikely! A -type f parameter would resolve it though.

...in case user has old DOS files
@s1eve-mcdichae1
Copy link
Contributor Author

s1eve-mcdichae1 commented May 22, 2025

technically if someone made a folder called duke3d.grp I think this would fail. I didn't think of it until now, but it's minor and unlikely! A -type f parameter would resolve it though.

if there is a folder there with that name, and this check fails, (ie. doesn't find a game data) then it tries to download shareware data and...what next?

Saves it to ports/duke3d/duke3d.grp/duke3d.grp right, which doesn't work anyway?

Edit no i don't know how it'll work with the unzip command:

unzip -L -o "$temp/dn3dsw13.shr" -d "$dest" duke3d.grp duke.rts

@joolswills
Copy link
Member

It's not a likely scenario - someone would have to make that folder. But maybe worth adding the -type f to be sure.

@joolswills
Copy link
Member

I think I missed your point - as even with the -type f, it would then not find the directory and try and unpack. and the unzip might fail. I guess this could happen in the old logic too. Your code is safer without the -type f so I'm going to merge.

@joolswills
Copy link
Member

Thank you!

@joolswills joolswills merged commit 53881d6 into RetroPie:master May 22, 2025
@s1eve-mcdichae1
Copy link
Contributor Author

as even with the -type f, it would then not find the directory and try and unpack. and the unzip might fail.

Yeah that's what I was getting at. Cheers!

@s1eve-mcdichae1 s1eve-mcdichae1 deleted the eduke32-game-data branch May 22, 2025 22:46
@joolswills
Copy link
Member

Sorry about that 😄. Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants