Recursively search CUSTOM folder for images (i.e. subdirectories)

[ZeldaZach]

Fix #2250.

This will allow for a recursive search through the CUSTOM folder until every image is checked to see if it matches the name of the card in question. If not found, continue like normal.

Checks subdirectories alphabetically, so if you have

.
├── Bar
│   └── example.jpg
└── Foo
    └── example.jpg

The Bar/example.jpg will be used.

[tooomm]

Should the same apply to customsets folder?

Maybe somebody wants to have folders for different games and such.

[Daenyth]

We should use this for custom sets too.

What happens if I put a symlink to the root directory inside custom? Will this then try to scan the whole file system?

[ZeldaZach]

I'll update it to include the custom sets dir, and true about symlinks. I can have it ignore them

[dev-id]

Does root take priority over subfolders?

[Nightfirecat]

@Daenyth @ZeldaZach I'd rather Cockatrice not ignore symlinks; I can see a use case for storing images on a separate drive/mount and symlinking the directory. I think that most anyone who's savvy enough to create symlinks is also smart enough not to symlink / (or otherwise recursive directories like ../) within the custom sets dir.

[Nightfirecat]

That said, that could also be addressed by adding some kind of directory traversal limit—just stop searching deeper after some number of directories, say 10.

[ZeldaZach]

I can try implementing a limit on how far down you can go @Nightfirecat, but do you think that's an optimal idea?

[ZeldaZach]

I took a second look at what I've done and what I feel should be done.
I see no reason to have sub-folders for the customsets folder. They're all XML databases, why do they need to be sub'd? It doesn't really make sense, nor does it need to be done...

Second, for symlinks I feel that we should allow them, but limit the depth to 4 (CUSTOM/symlink/dir1/dir2/dir3/image.jpg) so symlinks that are linked incorrectly don't jump everywhere...

Finally, I feel the current selection algorithm (alphabetically by directory/filename) is the best choice, but I'm open to hearing other ideas on how we should sort the CUSTOM folder.

[Daenyth]

I haven't had a chance to review code yet, but I'd really like to see logs on load time so that someone can read their debug log and see what cockatrice tried to find, and the results

[ZeldaZach]

@Daenyth Can you explain a little bit more on what you mean for the logs?

[Daenyth]

As you iterate through directories, log them

[Daenyth]

LGTM assuming it works right; I'd still like a qDebug or something in the while loop

[ZeldaZach]

I can put a qDebug if you'd like, but it's a bit wasteful imo.

[tooomm]

I see no reason to have sub-folders for the customsets folder. They're all XML databases, why do they need to be sub'd? It doesn't really make sense, nor does it need to be done...

What about that players who do other game as well, or several ones? Pokémon, Yu-Gi-Oh!, Hearthstone...
They might want a folder in customsets for each game to not mess with single pre-release xls's for certain sets or other custom xml's etc. which show up for each game individually. They might even favor that, and don't use a cards.xml at all?
You could also simply move the folder of the game you're about to play to customsets,

It doesn't hurt to have it anyway?

[tooomm]

What happens to "qDebug" and "doing the same in customsets folder"?!

[ctrlaltca]

If it's still relevant, open a new ticket describing the needed improvements; "doing the same in customsets folder" doesn't really belong to this PR anyway.
The PR was approved by @Daenyth and stuck for 38 days, i think that the user originally reporting the bug at #2250 will approve being finally able to use this feature.

[Daenyth]

Agreed on all counts

…

On Thu, Dec 8, 2016, 1:45 PM ctrlaltca ***@***.***> wrote: If it's still relevant, open a new ticket describing the needed improvements; "doing the same in customsets folder" doesn't really belong to this PR anyway. The PR was approved by @Daenyth <https://github.com/Daenyth> and stuck for 38 days, i think that the user originally reporting the bug at #2250 <#2250> will approve being finally able to use this feature. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2251 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA5NIDHt1PkEL3aVJCCqcJshTHtvb06ks5rGFBfgaJpZM4KknS0> .