OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

Fix `ResourceManager` path bug #325

Closed DanRStevens closed 4 years ago

DanRStevens commented 4 years ago

This is based on PR #323. Review may be easier if we first merge the parent branch, and then rebase this branch on master. Edit: Rebase complete.

This fixes #321. Provided wrapper methods may not exactly match those suggested in the issue. We may want to decide if more should be added, or if some should be removed.

This fixes #324. As I hadn't really made a definite decision on the final name, I went with the original I had suggested. I'd still be open to using an alternate name. Mostly I did this rename because it flowed naturally with the updates to the other issue above.

Brett208 commented 4 years ago

I'm trying to test this against OP2Archive.

auto filenames = XFile::GetFilenamesFromDirectory(tempDirectory);

I'm trying to replace the function above, but there doesn't seem to be a replacement function that does not require a regex string? Is this on purpose?

auto filenames = XFile::DirFiles(tempDirectory); // This doesn't work

-Brett

DanRStevens commented 4 years ago

You're right, that is a rather useful overload that is absent.

I suppose technically the old code would be equivalent to Dir(tempDirectory). However, I believe you want to specifically look for files only, not directories. Hence it would be more correct to use an alternate replacement method. This very much gets at the heart of the original debate about how we should return and filter results.

I'll add an overload to handle returning all files (but not directories) from a folder, without additional filtering.

Brett208 commented 4 years ago

Thanks. I'll keep testing once the new overload is pushed.

DanRStevens commented 4 years ago

Well that was quick :wink: