Author Topic: A better way of checking if a file has been successfully opened  (Read 4623 times)

borfast

  • Guest
A better way of checking if a file has been successfully opened
« on: November 29, 2010, 10:49:21 pm »
Hey there,

I finally decided to take a look at this and I must say I'm impressed! I didn't expect to find so many things already implemented.

But before I got it to work I faced a little problem: the game kept complaining that it "Failed to load palette, make sure X-Com is in the DATA subfolder."

Enter the command line:
Code: [Select]
rgrep -n "Failed to load palette" and there I go, src/Engine/Palette.cpp. By the way, this wasn't where I would find the solution to my problem; I was executing the openxcom binary file from the project root folder instead of the bin folder and the DATA directory location is hardcoded in StartState.cpp, which, by the way, I recommend to be fixed if you ever want the game to be installable. But I suppose that can be left for later on.

Before moving on, let me say that my C++ is a bit rusty, so don't just take my word for what I say is the best/correct way to do something.

That said, I noticed that you check if a file open operation was successful with
Code: [Select]
if (!palFile), which may not work as intended, since the constructor for std::ifstream creates the object even if it can't open the file you passed it, meaning palFile will always be an object when you verify it, and thus the condition will never fail.

When the constructor (or the open() method) can't open the file, there's a bit that gets set in the class, indicating that there was an error, and the internal file pointer remains unset.

Thus, the "correct" way to perform the check is to use the ifstream::fail() or ifstream::good() methods, which verify the bit mentioned above and return true or false accordingly. So the piece of code I mentioned would become something like this:
Code: [Select]
if (palFile.fail()) {
I didn't check to see if this is the way it is done elsewhere but assuming it is, changing it could save you some trouble.

Offline SupSuper

  • Lazy Developer
  • Administrator
  • Commander
  • *****
  • Posts: 2131
    • View Profile
Re: A better way of checking if a file has been successfully opened
« Reply #1 on: November 30, 2010, 02:42:22 pm »
fstream overloads the ! operator, so !palFile and palFile.fail() are equivalent. C++ is funky like that. :)

Regarding the DATA subfolder issue, AFAIK there's no cross-platform way of getting the path to "a subfolder of the path of the executable", instead of "a subfolder of the path of the working directory".
« Last Edit: December 01, 2010, 06:12:22 pm by SupSuper »

borfast

  • Guest
Re: A better way of checking if a file has been successfully opened
« Reply #2 on: December 01, 2010, 02:23:10 pm »
Ha, I knew it should be doing that (the overloaded operator). Otherwise it would never fail and I would never see the error.

As for the DATA folder path, no, there's no simple, cross-platform way of getting the executable directory. At most you can get the current working directory but even that isn't cross-platform, because getcwd() is the POSIX way of doing it but Microsoft decided it should change the name of its POSIX-compatible function to _getcwd() and leave getcwd() as-is in its libraries, so at the very least you have to check if you're running Windows or POSIX and use the correct funcion for that. This is something that I'd probably use a #define macro for.

It's such a simple and, I'd say, common operation, that I don't know why libraries such as SDL don't have a cross-platform way of doing it implemented already. Anyway, I gave it a try and managed to get it to work. I Attached the patch to this post, so you can try it.

The basic idea is to get the path to the executable from the program arguments (args[0]) and add the DATA folder to it. There are two possible scenarios for this: you can either launch the executable by specifying the full path (something like C:\Program Files\Games\OpenXCom\openxcom.exe or /usr/bin/openxcom), or by specifying the path relative to the current working directory (like ../bin/openxcom, assuming you were inside the src folder).
In the first case, we just need to strip the executable name from the path, append the DATA folder and we're done.
The second case requires a bit more tinkering but is quite simple; we just append the relative path used to launch the game, to the current working directory, use realpath() to get the absolute path from that, strip the executable name, add the DATA folder and there we go.

I made it just for testing, so there are a couple of things missing.
The first a is that I didn't include support for Windows but the changes should be minimal, such as checking for "C:" or equivalent, instead of checking just for "/", when determining if we have an absolute path to the executable.
The second is that this assumes that DATA will always be in the same folder as the executable, which is not the case if you want to package the game for distributing for most linux distributions, since the executable will be in /usr/bin or equivalent, and the data will be in /usr/share.

For now this solution works and allows me to do what I wanted, which is to launch the game from the src folder, because otherwise, I have to
cd ../bin
./openxcom
everytime I compile the game on the command line (this shouldn't be a problem if you're using an IDE such as CodeBlocks).
But later on, this needs to be thought of, if you want to allow the game to be distributed for Linux in a "correct" way.

Offline Daiky

  • Battlescape Programmer
  • Administrator
  • Commander
  • *****
  • Posts: 904
    • View Profile
Re: A better way of checking if a file has been successfully opened
« Reply #3 on: December 01, 2010, 03:14:06 pm »
If the data folder path is going to be a setting in the options file, wouldn't that be an absolute path then? That way the current working directory does not matter and you can have the executable and data in seperate folders if you want.


Offline michal

  • Commander
  • *****
  • Posts: 635
    • View Profile
Re: A better way of checking if a file has been successfully opened
« Reply #4 on: December 01, 2010, 03:27:05 pm »
But later on, this needs to be thought of, if you want to allow the game to be distributed for Linux in a "correct" way.

You mean distributed with Linux distributions? That won't happen soon, because OpenXcom needs nonfree resources.
Or do you mean some unnoficial packages?

Just out of curiosity - does your method works with symlinks?
« Last Edit: December 01, 2010, 03:30:02 pm by michal »

Offline SupSuper

  • Lazy Developer
  • Administrator
  • Commander
  • *****
  • Posts: 2131
    • View Profile
Re: A better way of checking if a file has been successfully opened
« Reply #5 on: December 01, 2010, 07:01:00 pm »
This is why I avoid cross-platform issues like the plague. It's a great concept in theory, until you inevitably run into stuff where you don't have some nice API to cover your ass. :P

The folder dilemma is the biggest one. Optimally I'd just ignore the whole thing and keep it all in one folder (that's what I'm doing so far). Simple, cheap, portable. But of course that's not the proper way to do things. Windows has Program Files, ProgramData, AppData, My Documents, etc. Linux has /usr/bin, /usr/share, /usr/doc, /home, etc. And I don't even know how Mac and other platforms do it! All with their own special APIs, installation procedures, etc. And I can't just cheat a bit and have a configuration file for the folders, because the proper way won't allow configuration and executable in the same place, so we're back where we started! Only now I have a headache. So can you blame me for being lazy? :P

Cross-platform stuff
I don't think every platform is guaranteed to put the executable path in args[0], though don't quote me on that.

If the data folder path is going to be a setting in the options file, wouldn't that be an absolute path then? That way the current working directory does not matter and you can have the executable and data in seperate folders if you want.
You still need to figure out the path at least once to populate the options file, unless you expect every user to manually fill it out. :P

You mean distributed with Linux distributions? That won't happen soon, because OpenXcom needs nonfree resources.
Or do you mean some unnoficial packages?
I think he just means following the standard Linux folder structure (instead of all files in one folder), whether through an unofficial package or "make install" or whatever.

Mind you there's always the users who do want to keep it all in one folder for portability. Dilemmas, dilemmas.
« Last Edit: December 01, 2010, 11:54:04 pm by SupSuper »

borfast

  • Guest
Re: A better way of checking if a file has been successfully opened
« Reply #6 on: December 01, 2010, 08:58:01 pm »
Yes, I meant following the standard Linux/Unix folder structure to allow for make install and such.

As for the executable path in args[0], you may be right, I think I've seen that somewhere.

Those who always want to keep it in the same folder can use the make install procedure, by previously passing --with-prefix to the configure script that generates the Makefile. Or you can keep it as a single package, meant to be run from one single folder, as it is now. :)

Anyway, this is getting beyond what I wanted it to go. It was a simple suggestion based on a small problem I faced, not something I think needs to be implemented right away.

Perhaps the best solution for now is to add a note to the documentation, about the need to execute the program from its own directory, so others won't have the same problem I did. No need for complicating things right away :)

Offline luciderous

  • Colonel
  • ****
  • Posts: 108
  • There is no spoon...
    • View Profile
Re: A better way of checking if a file has been successfully opened
« Reply #7 on: December 01, 2010, 09:53:33 pm »
I'm not sure yet how it might complicate building the Mac bundle, so I'd rather suggest to keep everything the way it is now - you know, don't try to fix what ain't broken and stuff  ;)