openage icon indicating copy to clipboard operation
openage copied to clipboard

Updating headers with std::filesystem for Win

Open simonsan opened this issue 4 years ago • 4 comments

There were some problems in the past regarding std::filesystem, MSVC 2017 and 2019 added support so we would be able to update our sources correspondingly.

std::filesystem for VS2017

simonsan avatar Apr 22 '20 09:04 simonsan

I would recommend to replace our custom path handling within libopenage/util/fslike/directory.h to use std::filesystem instead of our custom implementation. The frontent, our path and filesystem abstraction (openage/util/fslike/ and libopenage/util/fslike/) should stay and then be a wrapper around std::filesystem.

This is because our path system supports passing paths between C++ and Python, and std::filesystem can only be used for native C++ paths.

TheJJ avatar Apr 22 '20 10:04 TheJJ

I would recommend to replace our custom path handling within libopenage/util/fslike/directory.h to use std::filesystem instead of our custom implementation. The frontent, our path and filesystem abstraction (openage/util/fslike/ and libopenage/util/fslike/) should stay and then be a wrapper around std::filesystem.

This is because our path system supports passing paths between C++ and Python, and std::filesystem can only be used for native C++ paths.

I think with a bit of support I'll be able to tackle this one as my first contribution :)

sl1g18 avatar May 03 '20 22:05 sl1g18

I think with a bit of support I'll be able to tackle this one as my first contribution :)

Nice, and for sure we will be able to support ;-) As a starter you could read a small workflow recommendation by us so here: https://github.com/SFTtech/openage/blob/master/doc/contributing.md#workflow

For example just by making your Draft-PR early and marking it [WIP] we will be able to look easily into things, you and we can comment on lines of code with questions, feedback or explanations etc.

simonsan avatar May 03 '20 23:05 simonsan

heads up, though, is that std::filesystem is (or at least was) very crashy-crashy with msvc so I'd suggest a lot of testing before merging anything.

iirc I ended up blaming some internal refcounting in their path objects, it seemed to go away once I switched to always convert to strings in freeaoe (instead of storing std::filesystem::paths). aaand remember generic_string() vs string().

sandsmark avatar Aug 05 '20 18:08 sandsmark