ENIGMA Development Environment
Website is in read-only mode due to a recent attack.

C++17 temp_directory_path() broken on Windows

Reporter: time-killer-games  |  Status: open  |  Last Modified: March 26, 2021, 11:23:58 AM

I can tell Josh and fundies have a fetish for having broken c++17 features in enigma. emake broken.

std::ofstream egmlog(fs::temp_directory_path().string() + "/enigma_libegm.log", std::ofstream::out);

Also please use path.u8string() instead of path.string() to avoid creating more tech debt for when this code eventually works on english-only machines.

If you guys ever asked me to do code review, pitiful mistakes like these wouldn't be merged, because I actively use and test all our desktop platforms and value them as equals and not with a Linux bias/fetish.

fundies  
broken how? Is it crashing? Always or only if you pass a gmk? Also, you can review any PR you want they're all public
time-killer-games  

Running emake complains with an error of temp_directory_path failing because the c++17 devs never got it working on Windows. Last I checked you already knew this because we talked about it before and you confirmed it with Robert or whoever so it needs no further explanation

Pretty much prevents using emake completely.

fundies  

It's just a log file you shouldn't need it to work though
time-killer-games  

Yes but the fact it's even called prevents running emake. I believe this is probably due the fact you didn't use the no except version but I'll double check the docs

Edit:

Oddly enough there is no "no except" version in the docs but that's probably a problem with the docs being written correctly. It throws an exception and in my memory passing an error code argument will allow it to fail silently.

fundies  

Also afaict the CI is running empty games on windows fine. If it's just running projects that broke you can add a test for that pretty easy but I'd rather someone port the test harness to windows
time-killer-games  

It could be I may need a package update and they may have fixed it. I'll install master to verify. I was too quick to delete my install.

Of course I could just make test program as well outside of enigma

time-killer-games  

#include <iostream>
#include <filesystem>

int main() {
  std::cout << std::filesystem::temp_directory_path().string() << std::endl;
  return 0;
}

Capture

time-killer-games  

where as:

#include <iostream>
#include <filesystem>
#include <system_error>

int main() {
  std::error_code ec;
  std::cout << std::filesystem::temp_directory_path(ec).string() << std::endl;
  return 0;
}

Capture

This is why CI will not ever be as reliable as doing it yourself. The CI is a mere excuse to waste time writing CI instead of doing real tests.

time-killer-games  

The funny thing is, idk how the C++17 devs fuckered this one up, they claim it uses GetTempPath() on Windows underneath and that has always worked for me, I even use that as a workaround to this problem in my GM extensions.
time-killer-games  

And finally:

#include <iostream>
#include <windows.h>

int main() {
  wchar_t buffer[MAX_PATH + 1];
  if (GetTempPathW(MAX_PATH + 1, buffer)) {
    std::wcout << buffer << std::endl;
  }
  return 0;
}

Capture

time-killer-games  

And would you just look at that:

Capture

At least we learned a valuable lesson today. UTF-8 can still technically be supported if we add but two characters, "u8" in every fucking place you guys forgot to. I learned something too, update before whining shite broke.

time-killer-games  

well it's still broken even after the update. tf...

#include <iostream>
#include <filesystem>

int main() {
  std::cout << std::filesystem::temp_directory_path().string() << std::endl;
  return 0;
}

Capture

fundies  

I would talk to mingw people about it. However, feel free to implement your GetTempPathW in shared/filesystem and replace my call with your routine
time-killer-games  

Unfortunately that's a bit more involved than I have time for right now, because a lot of things like this require moving our widen() and narrow() functions to shared, which I've tried doing in a recent pr I ended up losing patience and closing it because of how many things that get broken by moving those which needed to be resolved.

I'll do it when I'm feeling less depressed some day. lel

Please sign in to post comments, or you can view this issue on GitHub.