Hard Light Productions Forums
Off-Topic Discussion => Programming => Topic started by: FreeSpaceFreak on March 09, 2011, 04:26:37 pm
-
I've been practising a little C++ lately - I already have some basic Java experience from university, but C++ seemed like a better language to learn (not in the least because of FSO! maybe one day...). Here's the code of my first useful program - it searches for all .fs2 files in its directory, reads every bit of text between XSTR-quotes, then writes all this to a textfile. For spellchecking purposes.
Now, since I have about one week of C++ experience so far, I'm probably doing some things terribly wrong here, and I'd like to know. Poor=performance code, cross-platform compatibility, etc. etc. Any (constructive) criticism you have would be most appreciated :)
(Note: this version of the code is outdated, an updated version is a few posts down)
#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <dirent.h>
#include <direct.h> // _getcwd
#include <cstdlib> // system("PAUSE")
using namespace std;
class Mtxt {
private:
string alltxt;
// Constructor, does all the hard work
public: Mtxt(string mname)
{
int max_strlen = 999999; //Max length of string temp
string temp; //contains text to be added
char temp2 [7]; //for mission parsing
int pointloc; //pointer location
// Open the mission file
ifstream curmis;
curmis.open (mname);
// Check if opening is successful
if (curmis.is_open())
{
// cout << "Mission " << mname << " successfully opened." << endl;
// Now try to read the mission
while (curmis.good())
{
pointloc = (int) curmis.tellg();
if (pointloc == -1)
{
cout << "Error reading file." << endl;
break;
}
//Read next series of characters
curmis.get(temp2,7);
//If at XSTR, get the string up to the closing quote; else go to next character
if( strcmp( temp2, "XSTR(\"") == 0)
{
getline(curmis,temp,'"');
alltxt.append(temp);
alltxt.append("\n");
}
else
{
curmis.seekg(pointloc + 1,ios_base::beg);
}
//If all that's left is a newline, go to next character
while( curmis.peek() == '\n' )
{
curmis.seekg(2,ios_base::cur);
}
//Append an additional newline between mission sections
/* This triggers for every hash, also e.g. table entries.
May want to fix that at some point, but it's just cosmetic. */
if (curmis.peek() == '#')
{
alltxt.append("\n");
}
}
// Close mission file
// cout << "Closing mission " << mname << endl << endl;
curmis.close();
}
else
cout << "Error opening mission " << mname << endl;
}
// Text getter
public: string getText()
{
return alltxt;
}
}; // Class definition has a trailing semicolon!
int main()
{
vector<string> mnames; //will contain all mission names
char workdir [999];
DIR *dir;
struct dirent *entry;
string strentry;
ofstream txt;
/*******************************************************************/
// Look for missions
_getcwd(workdir,999);
if( dir = opendir(workdir) )
{
while(entry = readdir(dir))
{
if( strcmp(entry->d_name, ".") != 0
&& strcmp(entry->d_name, "..") != 0)
{
/* entry -> d_name is always 261 characters
so cast to string*/
strentry = entry -> d_name;
// Only .fs2 files are interesting
if( strentry.find(".fs2") != -1)
{
// Found an .fs2 file! Now add it to the mnames vector.
mnames.push_back(strentry);
}
}
}
closedir(dir);
}
/*******************************************************************/
// Parse missions
txt.open("all.txt");
if (txt.is_open())
{
for (int i = 0; i < (int) mnames.size(); i++)
{
cout << "Working on mission " << mnames[i] << endl;
// Make mission text object for current mission
Mtxt cur_mtext (mnames[i]);
// Write the text to file
txt << cur_mtext.getText();
}
cout << "\nDone! " << flush;
}
else
{
cout << "Error opening output file" << endl;
}
system("PAUSE");
return 0;
}
-
I took a few semesters of C++ at school, and I remember absolutely nothing about classes, so you're already one up on me. :p
-
In C++, brackets are almost always done on a separate line. In addition, unlike Java, C++ does not specify public/private/protected for each individual item (thank god), and classes are defined as having everything as private by default. Hence, public: should be on its own line, and you should specify access "chunks" when declaring class members, as demonstrated below.
class bar
{
string private;
string blah;
public:
void thisispublic();
void foo();
char blar();
protected:
int derp;
int morederp;
bool derpy;
};
Note that some OOP fundamentalists claim that protected is the most evil keyword ever and you should never use it. I personally think they have their head up their ass and should go **** themselves, but that's your decision.
-
The biggest issue with protected (at least as i remember from java 3 months ago) is that it specifies a seldom required scheme of access. 99% of the time either a public or a private class will work well as long as the code is written around it. The idea with an Object Oriented language is to keep things as compartmentalized (read *in objects*) as much as possible, and protected is a scheme of access that lets you bypass that while appearing like you aren't.
On the other hand, as far as I'm concerned, if it makes sense to use protected in your specific program, go for it.
-
It doesn't seem sensible to use C++ for this task. It would be simpler and easier to use a scripting language.
(Style) Comments inline, some repetitions omitted:
#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <dirent.h>
#include <direct.h> // _getcwd
#include <cstdlib> // system("PAUSE")
using namespace std;
class Mtxt {
private:
string alltxt;
// Constructor, does all the hard work
^ Why?
public: Mtxt(string mname)
{
^ Why is this here?
int max_strlen = 999999; //Max length of string temp
^ static const? ^ why?
string temp; //contains text to be added
^ meaningless name
char temp2 [7]; //for mission parsing
^ magic number
^ Why mix string and char[]?
int pointloc; //pointer location
^don't abbreviate unless it's well known, meaningless comment
// Open the mission file
ifstream curmis;
^ abbreviations...
curmis.open (mname);
^ no space
// Check if opening is successful
^ Pointless comment
if (curmis.is_open())
{
// cout << "Mission " << mname << " successfully opened." << endl;
// Now try to read the mission
while (curmis.good())
{
pointloc = (int) curmis.tellg();
^ use C++ style cast (i.e. static_cast<int>(curmis.tellg())
if (pointloc == -1)
{
cout << "Error reading file." << endl;
break;
}
//Read next series of characters
curmis.get(temp2,7);
//If at XSTR, get the string up to the closing quote; else go to next character
if( strcmp( temp2, "XSTR(\"") == 0)
^ missing space, extra space
{
getline(curmis,temp,'"');
alltxt.append(temp);
alltxt.append("\n");
}
else
{
curmis.seekg(pointloc + 1,ios_base::beg);
}
//If all that's left is a newline, go to next character
while( curmis.peek() == '\n' )
{
curmis.seekg(2,ios_base::cur);
}
//Append an additional newline between mission sections
/* This triggers for every hash, also e.g. table entries.
May want to fix that at some point, but it's just cosmetic. */
if (curmis.peek() == '#')
{
alltxt.append("\n");
}
}
// Close mission file
^ Pointless
// cout << "Closing mission " << mname << endl << endl;
curmis.close();
}
else
cout << "Error opening mission " << mname << endl;
^ Don't skip braces; it won't end well.
}
// Text getter
public: string getText()
{
return alltxt;
}
}; // Class definition has a trailing semicolon!
int main()
{
vector<string> mnames; //will contain all mission names
char workdir [999];
DIR *dir;
struct dirent *entry;
string strentry;
ofstream txt;
/*******************************************************************/
// Look for missions
_getcwd(workdir,999);
if( dir = opendir(workdir) )
{
while(entry = readdir(dir))
{
if( strcmp(entry->d_name, ".") != 0
&& strcmp(entry->d_name, "..") != 0)
{
/* entry -> d_name is always 261 characters
so cast to string*/
strentry = entry -> d_name;
// Only .fs2 files are interesting
if( strentry.find(".fs2") != -1)
{
// Found an .fs2 file! Now add it to the mnames vector.
mnames.push_back(strentry);
}
}
}
closedir(dir);
}
/*******************************************************************/
// Parse missions
txt.open("all.txt");
if (txt.is_open())
{
for (int i = 0; i < (int) mnames.size(); i++)
{
cout << "Working on mission " << mnames[i] << endl;
// Make mission text object for current mission
Mtxt cur_mtext (mnames[i]);
// Write the text to file
txt << cur_mtext.getText();
}
cout << "\nDone! " << flush;
}
else
{
cout << "Error opening output file" << endl;
}
system("PAUSE");
^ Just don't
return 0;
}
In C++, brackets are almost always done on a separate line.
Citation needed.
-
Okay, that's useful feedback so far, thanks :)
Question: how do I fix the issue (http://www.hard-light.net/forums/index.php?topic=75003.msg1483293#msg1483293) of MSVCP100.dll missing? Adding it to the download seems like a bad idea.
-
Arguments about bracing style are pretty pointless.
I use:
bool MyObject::doThing()
{
do_stuff;
};Except in header files for trivial accessors, in which case I put the whole lot on one line:
bool getEnabled() const {return m_enabled;}- If it doesn't fit on one line, then it's not staying in the header because it's not trivial enough.
One reason for this is that in constructors I can lay out multiply-inherited base class stuff nicely without breaking style:
MyClass::MyClass()
: MyBaseA(),
MyBaseB()
{
do_my_stuff;
};
I also ensure that all loops and if - else structures etc use braces, even if they're not actually necessary. Saves issues later when the one-liner turns into two or a macro. It also helps in a source control diff as it means that you can see at a glance whether the change was to the predicate or the consequent/alternative.
Aside from that:
I would strongly suggest that you use separate header and body files. You may not have seen these yet, but they are pretty much essential to making non-trivial programs.
In general, the Constructor shouldn't really do anything much - just prepare the ground. It's there to build the object, not to process lots of data.
Either use std:string or don't. Mixing char[] and std::string really is asking for trouble - it will blow up quite spectacularly when the string stops being pure ASCII.
Variables called "temp" and "temp2" are usually foolish. Give them a descriptive name - even if they really are temporary scratch variables, they've still got a purpose, so name them appropriately.
Question: how do I fix the issue (http://www.hard-light.net/forums/index.php?topic=75003.msg1483293#msg1483293) of MSVCP100.dll missing? Adding it to the download seems like a bad idea.
This DLL is a requirement of your compiler and contains a load of standard stuff that pretty much all programs need.
You can force the compiler to include it inside the program executable - this is called "Static Linking". Have a look in your compiler documentation and online and you should be able to find out how to do that.
-
Okay, that's useful feedback so far, thanks :)
Question: how do I fix the issue (http://www.hard-light.net/forums/index.php?topic=75003.msg1483293#msg1483293) of MSVCP100.dll missing? Adding it to the download seems like a bad idea.
Link the redist like chief did in the other thread, staticly link the runtime (search your compilers help for the details on this, MSVC it should be under the linker options for the project file), include the one dll (which Microsoft has licensed you to do), or include the redist that chief linked to.
Arguments about bracing style are pretty pointless.
Yes, a better suggestion would be to pick a style and stick with it. The primary goal is to be consistent. The lack of consistency (in naming, code style, etc) is one of the primary reasons that the FSO code base is so hard to follow.
-
Okay, I think I got the DLL thing fixed, the EXE should now be statically linked.
I know I'm kinda stretching the purpose of a constructor here, but - of course I could make a separate readTexts() method out of it, and then call that from the constructor, but that would only make things more complicated, wouldn't it?
I'd much prefer to use strings instead of char[] , but according to the documentation (http://www.cplusplus.com/reference/iostream/ifstream/), only ifstream::get can get the next n characters out of the stream, and it only works with char[]. Or am I missing out on something?
Also, what about cross-platforminess? Or is that too much to handle for beginning coders?
Latest version:
#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <dirent.h>
#include <direct.h> // _getcwd
#include <algorithm> // sort
//#include <cstdlib> // system("PAUSE")
using namespace std;
class Mtxt {
private:
string alltxt;
// Constructor, does the hard work
public:
Mtxt(string mname)
{
string addthis; //contains text to be added
//string XSTR(" has seven characters, ends with a closing \0
//this is a char[] because ifstream::get() works like that
char parser [7];
int pointerloc;
// Open the mission file
ifstream curmis;
curmis.open (mname);
if (curmis.is_open())
{
// Now try to read the mission
while (curmis.good())
{
pointerloc = static_cast<int>(curmis.tellg());
if (pointerloc == -1)
{
cout << "Error reading file." << endl;
break;
}
//Read next series of characters
curmis.get(parser,7);
//Append tags where needed;
//skip Command briefing because currently
//indistinguishable from #Command (message source)
if (strcmp(parser,"#Brief") == 0)
{
alltxt.append("\nBRIEFING\n\n");
}
else if (strcmp(parser,"#Debri") == 0)
{
alltxt.append("\nDEBRIEFING\n\n");
}
else if (strcmp(parser,"#Objec") == 0)
{
alltxt.append("\nCARGO\n\n");
}
else if (strcmp(parser,"#Event") == 0)
{
alltxt.append("\nDIRECTIVES\n\n");
}
else if (strcmp(parser,"#Goals") == 0)
{
alltxt.append("\nGOALS\n\n");
}
else if (strcmp(parser,"#Messa") == 0)
{
alltxt.append("\nMESSAGES\n\n");
}
//If at XSTR, get the string up to the closing quote; else go to next character
else if (strcmp(parser, "XSTR(\"") == 0)
{
getline(curmis,addthis,'"');
if (addthis != "Nothing")
{
alltxt.append(addthis);
alltxt.append("\n");
}
}
else
{
curmis.seekg(pointerloc + 1,ios_base::beg);
}
//If all that's left is a newline, go to next character
while( curmis.peek() == '\n' )
{
curmis.seekg(2,ios_base::cur);
}
}
curmis.close();
}
else
{
cout << "Error opening mission " << mname << endl;
}
}
string getText()
{
return alltxt;
}
};
int main()
{
vector<string> mnames; //will contain all mission names
char workdir [999];
DIR *dir;
struct dirent *entry;
string strentry;
ofstream txt;
cout << "MTExPorter, v0.2 -- by FSF\n" << endl;
/*******************************************************************/
// Look for missions
_getcwd(workdir,999);
if( dir = opendir(workdir) )
{
while(entry = readdir(dir))
{
if( strcmp(entry->d_name, ".") != 0
&& strcmp(entry->d_name, "..") != 0)
{
//make string for better workability
strentry = entry -> d_name;
// Only .fs2 files are interesting
if( strentry.find(".fs2") != -1)
{
// Found an .fs2 file! Now add it to the mnames vector.
mnames.push_back(strentry);
}
}
}
closedir(dir);
}
// Sort missions
sort (mnames.begin(),mnames.end());
/*******************************************************************/
// Parse missions
txt.open("all.txt");
if (txt.is_open())
{
for (int i = 0; i < (int) mnames.size(); i++)
{
cout << "Parsing " << mnames[i] << endl;
// Make mission text object for current mission
Mtxt cur_mtext (mnames[i]);
// Write the text to file
txt << cur_mtext.getText() << "\n\nNEXT MISSION\n\n" << endl;
}
}
else
{
cout << "Error making output file" << endl;
}
//system("PAUSE");
return 0;
}
-
If it doesn't fit on one line, then it's not staying in the header because it's not trivial enough.
Unless you are building a template-ized class in which case the whole she-bang has to be in some sort of #include'd file.
Also, what about cross-platforminess? Or is that too much to handle for beginning coders?
That is a giant mess of spagettified monstrosities you do not want to touch for a long long time.
I'd much prefer to use strings instead of char[] , but according to the documentation, only ifstream::get can get the next n characters out of the stream, and it only works with char[]. Or am I missing out on something?
Strinsg have .c_str() that you can use to access their internal char[] array. Although, you should note that char[] and char* are the same thing, and in my experience [] is used to refer to static arrays, where in this case its a dynamic char* array. But that's semantics.
Yes, a better suggestion would be to pick a style and stick with it. The primary goal is to be consistent. The lack of consistency (in naming, code style, etc) is one of the primary reasons that the FSO code base is so hard to follow.
I wasn't supporting one bracing style over another - I tend to use the one that seems most appropriate for the situation. I was just pointing out that C++ tends towards having braces on separate lines, especially for classes and struct declarations.
I know I'm kinda stretching the purpose of a constructor here, but - of course I could make a separate readTexts() method out of it, and then call that from the constructor, but that would only make things more complicated, wouldn't it?
Do this. It'll save you headaches when you inevitably have like 5 or 6 different constructors who all have to do almost exactly the same thing except for a few minor changes.
99% of the time either a public or a private class will work well as long as the code is written around it.
This is my issue with people saying protected is evil. Is it technically bad practice to use protected? Yes. It breaks encapsulation. Is it PRACTICAL to put in Get/Set methods for EVERY SINGLE VARIABLE anyone would EVER HAVE TO ACCESS even when its an internal class structure used by the engine that no outside API would ever see? NO. It's a stupid waste of time that would be better spent making your program actually work then adhere to a bunch of ridiculous coding practices that need to be taken with a grain of salt before being applied. And if this actually makes your code more maintainable then you need to hire programmers that aren't idiots.
-
^ thus why I qualified it by dividing the reasoning behind not using protected from the practical, real world answer. That, yes, the reasoning behind not using protected is that it breaks encapsulation. At the same time, I agree, it is not always practical or necessary to follow that religiously.
If it isn't going to add significant amounts of code and difficulty to your code layout, try to not use protected. Otherwise, use it, just make sure you understand the scoping issues involved in the use of it.
-
Sounds like there's a lot of confusion about protected.
Protected is there for subclassing.
In pretty much every non-trivial inheritance tree there will be some stuff that *all* subclasses will want (or need) to be able to do - but that really shouldn't be available to things that aren't a subclass.
Private - Mine! Mine! my preciouussssss.
Protected - Only my subclasses can have this.
Public - Anybody can use this.
There are a lot of programs that you simply couldn't do without protected - you either end up writing the exact same method over and over (defeating the whole point), or you have to make things public that really shouldn't be available (and hence can break the application in the future).
I think you're both thinking of "Friends", which are simply not OOP. Avoid these when practical - they are never needed for an application to work as you can always do it another way - but friends may offer size and/or speed improvements in some circumstances.
-
Sounds like there's a lot of confusion about protected.
Protected is there for subclassing.
In pretty much every non-trivial inheritance tree there will be some stuff that *all* subclasses will want (or need) to be able to do - but that really shouldn't be available to things that aren't a subclass.
Private - Mine! Mine! my preciouussssss.
Protected - Only my subclasses can have this.
Public - Anybody can use this.
There are a lot of programs that you simply couldn't do without protected - you either end up writing the exact same method over and over (defeating the whole point), or you have to make things public that really shouldn't be available (and hence can break the application in the future).
I think you're both thinking of "Friends", which are simply not OOP. Avoid these when practical - they are never needed for an application to work as you can always do it another way - but friends may offer size and/or speed improvements in some circumstances.
See, this is what I use protected for. However, I have seen a lot of stupid people on the internet specifically declaring that the protected keyword is evil and that no one should ever use it, because it still technically breaks encapsulation, its just that anyone religiously adhering to that rule is completely missing the point of programming in the first place. Friends, obviously, are a much more egregious offender of OOP, but in some cases you want one class to have access to a variable and no other class for internal engine stuff, and building an entire seperated class structure is both impractical and makes the program structure more complicated and difficult to follow.
In general, I will always take the path that results in simpler, easier to understand code structure, since the vast majority of the time this also gives you performance benefits. Of course in other cases what I'm trying to do is just weird, so I'll go with the path of least resistance.
-
Here's the code of my first useful program - it searches for all .fs2 files in its directory, reads every bit of text between XSTR-quotes, then writes all this to a textfile. For spellchecking purposes.
This is a very useful program. :yes:
An even more useful program would be one that keeps track of the XSTR numbers, generates a new XSTR index for every XSTR that has an index of -1, and inserts this new index back into the original file. This would be a godsend for campaign and mod translators. :)
-
Or, basically, this (http://www.hard-light.net/forums/index.php?topic=70718.0)? m!m already did that :P
-
Wow, I forgot about that. :nervous:
But it can't hurt to have two of them, can it? :) And it'll help you improve your coding skills. :yes:
-
Understand what your options are and the pros and cons of each. Try to standardize and adopt (or create) conventions in your code as much as possible, but recognize when a specific situation warrants from breaking from convention.
For example: I always try to put {}s on their own line because it helps emphasize separate code blocks. But when there's a big list of logic statements with one line commands, I will prefer same-line brackets for readability.
if (strcmp(parser,"#Brief") == 0) {
alltxt.append("\nBRIEFING\n\n");
} else if (strcmp(parser,"#Debri") == 0) {
alltxt.append("\nDEBRIEFING\n\n");
} else if (strcmp(parser,"#Objec") == 0) {
alltxt.append("\nCARGO\n\n");
} else if (strcmp(parser,"#Event") == 0) {
alltxt.append("\nDIRECTIVES\n\n");
} else if (strcmp(parser,"#Goals") == 0) {
alltxt.append("\nGOALS\n\n");
} else if (strcmp(parser,"#Messa") == 0) {
alltxt.append("\nMESSAGES\n\n");
} else if (strcmp(parser, "XSTR(\"") == 0) {
//If at XSTR, get the string up to the closing quote; else go to next character
getline(curmis,addthis,'"');
if (addthis != "Nothing")
{
alltxt.append(addthis);
alltxt.append("\n");
}
} else {
curmis.seekg(pointerloc + 1,ios_base::beg);
}
What I like about your code:
- Good comments
- Checks for errors
- Not hard to read
This short list is not to be underestimated. These are fundamental important coding concepts that are very commonly forgotten, and which are very considerate to the people you're coding with.
Other thoughts:
- Make class names and public function names descriptive, since those are most likely what other people are using
- Avoid magic numbers. Use const variables or #defines. That gives a single place that they can change it that will also automatically update "curmis.get(parser,7);"
- Separate class member definitions from member declarations. Don't worry about inlining a function as big as Mtext.
- Put class declarations in a .hpp file with include guards
- Use a consistent capitalization convention for functions, and a consistent (preferably different) capitalization convention for variables. Classes can go with either one.
- Differentiate class member variables from local variables with some kind of convention ("m_*" is common)
- Use references and const where you can (so the compiler does not need to copy - see GetText() in below example).
- Don't do stuff that can fail in the constructor (ie open a file)
- Use if(error){return} rather than if(good){work}else{return} to reduce indenting
- Don't use C casts like (int). Use C++ casts like static cast. Also, prefer the right type of the variable.
- Use cout for output, cerr for errors
I didn't compile the below stuff and only fixed some obvious errors. But this is to give you an idea of how I personally would restructure it.
MissionText.hpp
#ifndef _MISSIONTEXT_HPP_
#define _MISSIONTEXT_HPP_
#include <string>
class MissionText
{
private:
std::string m_allText;
public:
MissionText();
//Load a mission file; false on failure
bool Load(std::string const &mname);
std::string const &GetText() const
{ return m_allText; }
};
#endif /* _MISSIONTEXT_HPP_ */
MissionText.cpp
#include "MissionText.hpp"
#include <iostream>
#include <fstream>
#include <string>
//#include <cstdlib> // system("PAUSE")
using namespace std;
static int const gsc_parserLength = 7;
MissionText::MissionText()
{
}
bool MissionText::Load(string mname)
{
string addthis; //contains text to be added
//string XSTR(" has seven characters, ends with a closing \0
//this is a char[] because ifstream::get() works like that
char parser [gsc_parserLength];
int pointerloc;
// Open the mission file
ifstream curmis;
curmis.open (mname);
if(!curmis.is_open())
{
cout << "Error opening mission " << mname << endl;
return false;
}
// Now try to read the mission
while (curmis.good())
{
pointerloc = static_cast<int>(curmis.tellg());
if (pointerloc == -1)
{
cerr << "Error reading file." << endl;
curmis.close();
return false;
}
//Read next series of characters
curmis.get(parser,gsc_parserLength);
//Append tags where needed;
//skip Command briefing because currently
//indistinguishable from #Command (message source)
if (strcmp(parser,"#Brief") == 0) {
m_allText.append("\nBRIEFING\n\n");
} else if (strcmp(parser,"#Debri") == 0) {
m_allText.append("\nDEBRIEFING\n\n");
} else if (strcmp(parser,"#Objec") == 0) {
m_allText.append("\nCARGO\n\n");
} else if (strcmp(parser,"#Event") == 0) {
m_allText.append("\nDIRECTIVES\n\n");
} else if (strcmp(parser,"#Goals") == 0) {
m_allText.append("\nGOALS\n\n");
} else if (strcmp(parser,"#Messa") == 0) {
m_allText.append("\nMESSAGES\n\n");
} else if (strcmp(parser, "XSTR(\"") == 0) {
//If at XSTR, get the string up to the closing quote; else go to next character
getline(curmis,addthis,'"');
if (addthis != "Nothing")
{
m_allText.append(addthis);
m_allText.append("\n");
}
} else {
curmis.seekg(pointerloc + 1,ios_base::beg);
}
//If all that's left is a newline, go to next character
while( curmis.peek() == '\n' )
{
curmis.seekg(2,ios_base::cur);
}
}
curmis.close();
return true;
}
main.cpp
#include "MissionText.hpp"
#include <algorithm> // sort
#include <dirent.h>
#include <string>
#include <vector>
using namespace std;
int main()
{
vector<string> mnames; //will contain all mission names
char workdir [999];
DIR *dir;
struct dirent *entry;
string strentry;
ofstream txt;
cout << "MTExPorter, v0.2 -- by FSF\n" << endl;
/*******************************************************************/
// Look for missions
_getcwd(workdir,999);
if( dir = opendir(workdir) )
{
while(entry = readdir(dir))
{
if( strcmp(entry->d_name, ".") != 0
&& strcmp(entry->d_name, "..") != 0)
{
//make string for better workability
strentry = entry -> d_name;
// Only .fs2 files are interesting
if( strentry.find(".fs2") != -1)
{
// Found an .fs2 file! Now add it to the mnames vector.
mnames.push_back(strentry);
}
}
}
closedir(dir);
}
// Sort missions
sort (mnames.begin(),mnames.end());
/*******************************************************************/
// Parse missions
txt.open("all.txt");
if (!txt.is_open())
{
cerr << "Error making output file" << endl;
return 1;
}
Mtxt cur_mtext;
for (size_t i = 0; i < mnames.size(); i++)
{
cout << "Parsing " << mnames[i] << endl;
// Make mission text object for current mission
if(cur_mtext.load(mnames[i]))
{
// Write the text to file
txt << cur_mtext.GetText() << "\n\nNEXT MISSION\n\n" << endl;
}
}
//system("PAUSE");
return 0;
}
It might seem silly to split this up into separate files this but as you get to larger applications this will become useful.
Psychologically, it's also good to separate things so that you start thinking of a class's behavior by its external interface, rather than its internal behavior. If you rely on knowledge of a class's internal workings, you will very quickly get overwhelmed when dealing with large code.
-
Okay, noted - I'll have to look into the #ifndef and #define stuff.
-
#ifndef _MISSIONTEXT_HPP_
#define _MISSIONTEXT_HPP_
// HEADER FILE STUFF FOR MISSIONTEXT GOES HERE
#endif /* _MISSIONTEXT_HPP_ */
This is here to make sure that your application doesn't include the header file more than once.
Say you've got a big application, and this header file defines a class that gets used all over the place.
If you don't use this structure, the second time it gets #included then the compiler will complain that you're trying to redefine an existing class - when actually you meant that you want to be able to use the class, not knowing whether or not some other class added it to the program already.
-
Oh, so that's what it's for! Umm, do only .h files need this, or .cpp files as well? Or does it depend on whether or not you define a class in the specific file?
I've been trying to imitate the Linux game Robots - for those still interested, here's the source of the first alpha. Am I doing it right with the splitting up into files?
Robots.h:
#ifndef _ROBOTS_H_
#define _ROBOTS_H_
bool newGame();
void drawHor(int Xmax);
char getInput();
int moveX(int inchar);
int moveY(int inchar);
#endif // _ROBOTS_H_
Robots.cpp:
#include "Robots.h"
#include "Things.cpp"
#include <iostream>
#include <vector>
using namespace std;
int newGame(int level)
{
/************** Initialize level **************/
//playfield size; coords counting from top left
const int XMAX = 60;
const int YMAX = 21;
const int NUMBOTS = 10*level;
vector<Robot> Bots; //All robots
vector<Pile> Scrap; //All scrapheaps
char drawthis = ' '; //What to draw?
char inchar [] = {'\0', '\0'}; //Player input
bool in_good = false; //for giving player another chance at faulty input
/************** Place player & bots **************/
Player You (XMAX/2, YMAX/2);
while (static_cast<int> (Bots.size()) < NUMBOTS)
{
bool badbot = true;
Robot MakeBot (1, 1);
while (badbot)
{
MakeBot.setXY (1 + rand() % XMAX, 1 + rand() % YMAX);
//Check if on top of the player
badbot = MakeBot.collide(You);
//Check if on top of another robot
for (int j = 1; j < static_cast<int> (Bots.size()); j++)
{
badbot = MakeBot.collide(Bots[j]);
}
}
Bots.push_back(MakeBot);
}
/************** Play game **************/
do
{
cout << "Level: " << level << " - Bots remaining: " << Bots.size() << endl;
//Show situation
drawHor (XMAX);
for (int j = 1; j <= YMAX; j++) // vertical (y-) coordinate
{
cout << '|';
for (int i = 1; i <= XMAX; i++) // horizontal (x-) coordinate
{
//Check what's here:
//A robot?
for (int k = 0; k < static_cast<int>(Bots.size()); k++)
{
if ( Bots[k].isHere(i,j) )
{
drawthis = '+';
}
}
//A scrapheap?
for (int k = 0; k < static_cast<int>(Scrap.size()); k++)
{
if ( Scrap[k].isHere(i,j) )
{
drawthis = 'X';
}
}
//The player?
if (You.isHere(i,j) )
{
drawthis = '@';
}
cout << drawthis;
drawthis = ' ';
}
cout << "|" << endl;
}
drawHor (XMAX);
//Player won?
if (Bots.size() == 0)
{
cout << "Congratulations! You beat this level! Continue? (y/n)";
inchar[0] = getInput();
if (strcmp(inchar,"n") == 0) {
return 0;
}
else {
return ++level;
}
}
//player input
in_good = false;
while (!in_good)
{
cout << "Enter a character and press Enter: ";
inchar[0] = getInput();
if (strcmp(inchar,"*") == 0)
{
bool bad_position = true;
int newx, newy;
while (bad_position)
{
newx = 1 + rand() % XMAX;
newy = 1 + rand() % YMAX;
// Check collision with scrapheaps
bad_position = false;
for (int l = 0; l < static_cast<int>(Scrap.size()); l++)
{
if (Scrap[l].isHere(newx,newy))
{
bad_position = true;
}
}
}
You.setX(newx);
You.setY(newy);
in_good = true;
}
else if (strcmp(inchar,"+") == 0)
{
cout << "Safe teleporting not implemented yet!" << endl;
}
else if (strcmp(inchar,"w") == 0)
{
cout << "Waiting doesn't really look good in the command-line, I'm afraid." << endl;
cout << "You'll have to do it the old-fashioned way by pressing 5." << endl;
}
else if (strcmp(inchar,"q") == 0)
{
cout << "Really quit? (y/n) ";
inchar[0] = getInput();
if (strcmp(inchar,"y") == 0)
{
return false;
}
}
else
{
int newx = You.getX() + moveX(atoi(inchar));
int newy = You.getY() + moveY(atoi(inchar));
// Check collision with scrapheaps
in_good = true;
for (int l = 0; l < static_cast<int>(Scrap.size()); l++)
{
if (Scrap[l].isHere(newx,newy))
{
// not using in_good = !Scrap[l].isHere
// because that would override earlier bots
in_good = false;
}
}
if (in_good)
{
You.setXY (newx, newy);
}
}
}
//check if player out of bounds
if (You.getX() > XMAX){
You.setX(XMAX);
}
if (You.getX() < 1){
You.setX(1);
}
if (You.getY() > YMAX){
You.setY(YMAX);
}
if (You.getY() < 1){
You.setY(1);
}
//First update all bots
for (int k = 0; k < static_cast<int>(Bots.size()); k++)
{
Bots[k].update(You.getX(), You.getY());
}
//Then check if something happens
for (int k = 0; k < static_cast<int>(Bots.size()); k++)
{
//Bot caught player!
if (Bots[k].collide(You))
{
cout << "\nGame over! Try again? (y/n)";
inchar[0] = getInput();
if (strcmp(inchar,"n") == 0) {
return 0;
}
else {
return 1;
}
}
else
{
// Check collision with other bots
for (int l = 0; l < static_cast<int>(Bots.size()); l++)
{
if (k != l && Bots[k].collide(Bots[l]))
{
// Add scrapheap, set robots for removal
Pile MakePile (Bots[k].getX(),Bots[l].getY());
Scrap.push_back(MakePile);
Bots[k].setRemove(true);
Bots[l].setRemove(true);
}
}
// Check collision with scrapheaps
for (int l = 0; l < static_cast<int>(Scrap.size()); l++)
{
if (Bots[k].collide(Scrap[l]))
{
Bots[k].setRemove(true);
}
}
}
}
//Remove all colliding bots
for (int k = Bots.size()-1; k >= 0; k--)
{
if (Bots[k].getRemove())
{
Bots.erase(Bots.begin() + k);
}
}
}
while (true);
return 1;
}
void drawHor (int Xmax)
{
for (int i = 0; i <= Xmax+1; i++)
{
cout << "=";
}
cout << endl;
}
char getInput()
{
/* old version didn't work
char input [2] = {'\0', '\0'}; //Input character + null for ending
char garb; //Everything else the player enters
while (input[0] == '\0') //Make sure to actually get input
{
cout << " Give input: ";
cin.get(input,2); //Get the first character
while ( cin.get ( garb ) && garb != '\n' ); //Purge input stream
cout << "Stream purged!" << endl;
}
return input[0];
*/
char input = '\0'; //Input character
char garb; //Everything else the player enters
do
{
cin.get (input); //Get the next character
}
while (input == '\0' || input == '\n'); //Make sure to actually get input
while (cin.get (garb) && garb != '\n' ); //Purge input stream
return input;
}
int moveX (int inchar)
{
switch (inchar)
{ // return (xmove)
case 1: {
return (-1);}
case 3: {
return (+1);}
case 4: {
return (-1);}
case 6: {
return (+1);}
case 7: {
return (-1);}
case 9: {
return (+1);}
default: {
return (0);}
}
}
int moveY (int inchar)
{
switch (inchar)
{ // return (ymove)
case 1: {
return (+1);}
case 2: {
return (+1);}
case 3: {
return (+1);}
case 7: {
return (-1);}
case 8: {
return (-1);}
case 9: {
return (-1);}
default: {
return(0); }
}
}
int main()
{
int level = 1;
while (level > 0)
{
level = newGame(level);
}
}
Things.cpp:
class Thing
{
protected:
int m_x, m_y;
public:
Thing ();
Thing (int xin, int yin)
{
m_x = xin;
m_y = yin;
}
int getX() {return m_x;}
int getY() {return m_y;}
void setX (int newX) {m_x = newX;}
void setY (int newY) {m_y = newY;}
void setXY (int newx, int newy){
m_x = newx; m_y = newy;}
void moveXY (int movex, int movey){
m_x += movex; m_y += movey;}
bool isHere (int isx, int isy)
{
if (m_x == isx && m_y == isy)
{
return true;
}
else
{
return false;
}
}
bool collide (Thing otherthing)
{
if (m_x == otherthing.getX() && m_y == otherthing.getY())
{
return true;
}
else
{
return false;
}
}
};
class Robot : public Thing
{
protected:
bool remove_this;
public:
Robot () : Thing () {};
Robot (int xin, int yin) : Thing(xin, yin)
{
remove_this = false;
}
void update (int playerx, int playery)
{
//update x-location
if (playerx > m_x) {
m_x++;
}
else if (playerx < m_x) {
m_x--;
}
//update y-location
if (playery > m_y) {
m_y++;
}
else if (playery < m_y) {
m_y--;
}
}
void setRemove (bool in) {remove_this = in;}
bool getRemove () {return remove_this;}
};
class Player : public Thing
{
public:
Player (int xin, int yin) : Thing(xin, yin) {}
//void update (int inchar)
//{
//}
};
class Pile : public Thing
{
public:
Pile (int xin, int yin) : Thing (xin,yin){};
};
-
I've been trying to imitate the Linux game Robots - for those still interested, here's the source of the first alpha. Am I doing it right with the splitting up into files?
Yes and no. In no particular order:
- Class Thing should have its own header file, and strictly speaking so should all of the child classes.
- Code files should be use all lowercase and underscores (_) because most compilers are case sensitive (VC being the notable exception, though it even has its case sensitive moments).
- The return type of a vector is a size_t not an int (contrary to how the FSO code base handles them) so don't cast size_t to int without a good reason. From what I see there isn't a good one (even FSO doesn't have a good reason other than the :v: coders decided that using negative numbers was good for error codes (which honestly has better alternatives)).
- main should not be with your application logic.
- Your main says that you return a value, make sure that you do what you promise.
- Strictly main also has two more arguments (int argc, char ** argv).
- When expecting input you should either say what is expected or offer help at the prompt.
- C++ headers techinically have a .hpp extenstion.
- The following files compile and run successfully on my machine.
- The code in robots can also be an object that main instantiates then calls a runGame().
- System headers should be first, followed by your global headers, then your file specific headers. This is so you don't mess up the system headers with your own symbols (there are times that you will want or need to, but an application at this level (IIRC even FSO doesn't mess with the system headers) does not need anything like that).
- using namespace std; is a crutch that will get you into a lot of trouble as you get into bigger projects. Don't get me wrong its not that using namespace is useless, the problem is the std namespace is pretty uncommon once you get away from simple projects like this.
Also, you should never1 #include a .cpp. This is because .cpps contain an implementation. You only need one copy of the implementation. On the other hand, you probably included the .cpp because otherwise it wouldn't like, correct? Since you seem to be Visual Studio all you should need to do is make sure that all of the .cpp files are in the project and VS will take care of calling the compiler correctly to make everything needs to link. That being said, do to it manually (I am assuming Visual C again)
cl /EHsc /out:robots.exe robots.cpp things.cpp main.cpp
#ifndef _ROBOTS_H_
#define _ROBOTS_H_
int newGame(int level);
void drawHor(int Xmax);
char getInput();
int moveX(int inchar);
int moveY(int inchar);
#endif // _ROBOTS_H_#include <iostream>
#include <vector>
#include "robots.hpp"
#include "things.hpp"
int newGame(int level)
{
/************** Initialize level **************/
//playfield size; coords counting from top left
const int XMAX = 60;
const int YMAX = 21;
const int NUMBOTS = 10*level;
std::vector<Robot> Bots; //All robots
std::vector<Pile> Scrap; //All scrapheaps
char drawthis = ' '; //What to draw?
char inchar [] = {'\0', '\0'}; //Player input
bool in_good = false; //for giving player another chance at faulty input
/************** Place player & bots **************/
Player You (XMAX/2, YMAX/2);
while (static_cast<int> (Bots.size()) < NUMBOTS)
{
bool badbot = true;
Robot MakeBot(1, 1);
while (badbot)
{
MakeBot.setXY (1 + rand() % XMAX, 1 + rand() % YMAX);
//Check if on top of the player
badbot = MakeBot.collide(You);
//Check if on top of another robot
for (int j = 1; j < static_cast<int> (Bots.size()); j++)
{
badbot = MakeBot.collide(Bots[j]);
}
}
Bots.push_back(MakeBot);
}
/************** Play game **************/
do
{
std::cout << "Level: " << level << " - Bots remaining: " << Bots.size() << std::endl;
//Show situation
drawHor (XMAX);
for (int j = 1; j <= YMAX; j++) // vertical (y-) coordinate
{
std::cout << '|';
for (int i = 1; i <= XMAX; i++) // horizontal (x-) coordinate
{
//Check what's here:
//A robot?
for (int k = 0; k < static_cast<int>(Bots.size()); k++)
{
if ( Bots[k].isHere(i,j) )
{
drawthis = '+';
}
}
//A scrapheap?
for (int k = 0; k < static_cast<int>(Scrap.size()); k++)
{
if ( Scrap[k].isHere(i,j) )
{
drawthis = 'X';
}
}
//The player?
if (You.isHere(i,j) )
{
drawthis = '@';
}
std::cout << drawthis;
drawthis = ' ';
}
std::cout << "|" << std::endl;
}
drawHor (XMAX);
//Player won?
if (Bots.size() == 0)
{
std::cout << "Congratulations! You beat this level! Continue? (y/n)";
inchar[0] = getInput();
if (strcmp(inchar,"n") == 0) {
return 0;
}
else {
return ++level;
}
}
//player input
in_good = false;
while (!in_good)
{
std::cout << "Enter a character and press Enter: ";
inchar[0] = getInput();
if (strcmp(inchar,"*") == 0)
{
bool bad_position = true;
int newx, newy;
while (bad_position)
{
newx = 1 + rand() % XMAX;
newy = 1 + rand() % YMAX;
// Check collision with scrapheaps
bad_position = false;
for (int l = 0; l < static_cast<int>(Scrap.size()); l++)
{
if (Scrap[l].isHere(newx,newy))
{
bad_position = true;
}
}
}
You.setX(newx);
You.setY(newy);
in_good = true;
}
else if (strcmp(inchar,"+") == 0)
{
std::cout << "Safe teleporting not implemented yet!" << std::endl;
}
else if (strcmp(inchar,"w") == 0)
{
std::cout << "Waiting doesn't really look good in the command-line, I'm afraid." << std::endl;
std::cout << "You'll have to do it the old-fashioned way by pressing 5." << std::endl;
}
else if (strcmp(inchar,"q") == 0)
{
std::cout << "Really quit? (y/n) ";
inchar[0] = getInput();
if (strcmp(inchar,"y") == 0)
{
return false;
}
}
else
{
int newx = You.getX() + moveX(atoi(inchar));
int newy = You.getY() + moveY(atoi(inchar));
// Check collision with scrapheaps
in_good = true;
for (int l = 0; l < static_cast<int>(Scrap.size()); l++)
{
if (Scrap[l].isHere(newx,newy))
{
// not using in_good = !Scrap[l].isHere
// because that would override earlier bots
in_good = false;
}
}
if (in_good)
{
You.setXY (newx, newy);
}
}
}
//check if player out of bounds
if (You.getX() > XMAX){
You.setX(XMAX);
}
if (You.getX() < 1){
You.setX(1);
}
if (You.getY() > YMAX){
You.setY(YMAX);
}
if (You.getY() < 1){
You.setY(1);
}
//First update all bots
for (int k = 0; k < static_cast<int>(Bots.size()); k++)
{
Bots[k].update(You.getX(), You.getY());
}
//Then check if something happens
for (int k = 0; k < static_cast<int>(Bots.size()); k++)
{
//Bot caught player!
if (Bots[k].collide(You))
{
std::cout << "\nGame over! Try again? (y/n)";
inchar[0] = getInput();
if (strcmp(inchar,"n") == 0) {
return 0;
}
else {
return 1;
}
}
else
{
// Check collision with other bots
for (int l = 0; l < static_cast<int>(Bots.size()); l++)
{
if (k != l && Bots[k].collide(Bots[l]))
{
// Add scrapheap, set robots for removal
Pile MakePile (Bots[k].getX(),Bots[l].getY());
Scrap.push_back(MakePile);
Bots[k].setRemove(true);
Bots[l].setRemove(true);
}
}
// Check collision with scrapheaps
for (int l = 0; l < static_cast<int>(Scrap.size()); l++)
{
if (Bots[k].collide(Scrap[l]))
{
Bots[k].setRemove(true);
}
}
}
}
//Remove all colliding bots
for (int k = Bots.size()-1; k >= 0; k--)
{
if (Bots[k].getRemove())
{
Bots.erase(Bots.begin() + k);
}
}
}
while (true);
return 1;
}
void drawHor (int Xmax)
{
for (int i = 0; i <= Xmax+1; i++)
{
std::cout << "=";
}
std::cout << std::endl;
}
char getInput()
{
/* old version didn't work
char input [2] = {'\0', '\0'}; //Input character + null for ending
char garb; //Everything else the player enters
while (input[0] == '\0') //Make sure to actually get input
{
cout << " Give input: ";
cin.get(input,2); //Get the first character
while ( cin.get ( garb ) && garb != '\n' ); //Purge input stream
cout << "Stream purged!" << endl;
}
return input[0];
*/
char input = '\0'; //Input character
char garb; //Everything else the player enters
do
{
std::cin.get(input); //Get the next character
}
while (input == '\0' || input == '\n'); //Make sure to actually get input
while (std::cin.get (garb) && garb != '\n' ); //Purge input stream
return input;
}
int moveX (int inchar)
{
switch (inchar)
{ // return (xmove)
case 1: {
return (-1);}
case 3: {
return (+1);}
case 4: {
return (-1);}
case 6: {
return (+1);}
case 7: {
return (-1);}
case 9: {
return (+1);}
default: {
return (0);}
}
}
int moveY (int inchar)
{
switch (inchar)
{ // return (ymove)
case 1: {
return (+1);}
case 2: {
return (+1);}
case 3: {
return (+1);}
case 7: {
return (-1);}
case 8: {
return (-1);}
case 9: {
return (-1);}
default: {
return(0); }
}
}
#include "things.hpp"
bool Thing::isHere (int isx, int isy)
{
if (m_x == isx && m_y == isy)
{
return true;
}
else
{
return false;
}
}
bool Thing::collide (Thing otherthing)
{
if (m_x == otherthing.getX() && m_y == otherthing.getY())
{
return true;
}
else
{
return false;
}
}
void Robot::update (int playerx, int playery)
{
//update x-location
if (playerx > m_x) {
m_x++;
}
else if (playerx < m_x) {
m_x--;
}
//update y-location
if (playery > m_y) {
m_y++;
}
else if (playery < m_y) {
m_y--;
}
}
//void Player::update (int inchar)
//{
//}#ifndef THINGS_HPP
#define THINGS_HPP
class Thing
{
protected:
int m_x, m_y;
public:
Thing(): m_x(0), m_y(0) {}
Thing(int xin, int yin)
:m_x(xin), m_y(yin) {}
int getX() {return m_x;}
int getY() {return m_y;}
void setX (int newX) {m_x = newX;}
void setY (int newY) {m_y = newY;}
void setXY (int newx, int newy){
m_x = newx; m_y = newy;}
void moveXY (int movex, int movey){
m_x += movex; m_y += movey;}
bool isHere (int isx, int isy);
bool collide (Thing otherthing);
};
class Robot : public Thing
{
protected:
bool m_remove_this;
public:
Robot () : Thing () {};
Robot (int xin, int yin):
Thing(xin, yin),m_remove_this(false) {}
void update (int playerx, int playery);
void setRemove (bool in) {m_remove_this = in;}
bool getRemove () {return m_remove_this;}
};
class Player : public Thing
{
public:
Player (int xin, int yin) : Thing(xin, yin) {}
//void update (int inchar);
};
class Pile : public Thing
{
public:
Pile (int xin, int yin) : Thing (xin,yin){};
};
#endif
#include "robots.hpp"
int main()
{
int level = 1;
while (level > 0)
{
level = newGame(level);
}
return 0;
}
1) The major exception would be when you are including boiler-plate type code. WxWidgets has a few examples of this, though even they are there because of legacy, trying to work around broken compilers not because they are strictly necessary.
-
The return type of a vector is a size_t not an int (contrary to how the FSO code base handles them) so don't cast size_t to int without a good reason. From what I see there isn't a good one (even FSO doesn't have a good reason other than the :v: coders decided that using negative numbers was good for error codes (which honestly has better alternatives)).
But then how to compare the size_t with a regular int? VC++ doesn't like if I don't cast it.
Strictly main also has two more arguments (int argc, char ** argv).
Are these the command-line arguments? Should I include support for them even if they're not used anywhere?
-
The return type of a vector is a size_t not an int (contrary to how the FSO code base handles them) so don't cast size_t to int without a good reason. From what I see there isn't a good one (even FSO doesn't have a good reason other than the :v: coders decided that using negative numbers was good for error codes (which honestly has better alternatives)).
But then how to compare the size_t with a regular int? VC++ doesn't like if I don't cast it.
Yes all compilers don't, but there is no reason that you can't declare the counter as a size_t rather than an int. Something like the below.
//A robot?
for (size_t k = 0; k < Bots.size(); k++)
{
if ( Bots[k].isHere(i,j) )
{
drawthis = '+';
}
}
Obviously there will be times that it is necessary to mix a size_t and an int in a comparison and at that time make sure that you check that the numbers are in a valid range to compare with each other. That is, make sure that the int is not negative and that the size_t is not above MAX_INT (2^31, which is 2,147,483,647, in 32-bit mode), otherwise you get funky things happen like below.
The size of int is 4
The size of size_t (unsigned int) is 4
** The number in [] is the value that is written in the code.
1[ 1] == 1[ 1] == true (Good, true)
1[ 1] > 1[ 1] == false (Good, false)
1[ 1] < 1[ 1] == false (Good, false)
2[ 2] > 1[ 1] == true (Good, true)
-1[ -1] < 1[ 1] == false (Fail, true)
-1[ -1] > 1[ 1] == true (Fail, false)
1[ 1] < 4294967295[ -1] == true (Fail, false)
1[ 1] > 4294967295[ -1] == false (Fail, true)
1[ 1] == 4294967295[ -1] == false (Good, false)
2147483646[ 2147483646] == 2147483646[ 2147483646] == true (Good, true)
2147483647[ 2147483647] == 2147483647[ 2147483647] == true (Good, true)
-2147483648[ 2147483648] == 2147483648[ 2147483648] == true (Good, true)
-2147483648[-2147483648] == 2147483648[ 2147483648] == true (Fail, false)
-2147483647[ 2147483649] == 2147483649[ 2147483649] == true (Good, true)
-2147483646[ 2147483650] == 2147483650[ 2147483650] == true (Good, true)
The size of int is 4
The size of size_t (unsigned int) is 8
** The number in [] is the value that is written in the code.
1[ 1] == 1[ 1] == true (Good, true)
1[ 1] > 1[ 1] == false (Good, false)
1[ 1] < 1[ 1] == false (Good, false)
2[ 2] > 1[ 1] == true (Good, true)
-1[ -1] < 1[ 1] == false (Fail, true)
-1[ -1] > 1[ 1] == true (Fail, false)
1[ 1] < 4294967295[ -1] == true (Fail, false)
1[ 1] > 4294967295[ -1] == false (Fail, true)
1[ 1] == 4294967295[ -1] == false (Good, false)
2147483646[ 2147483646] == 2147483646[ 2147483646] == true (Good, true)
2147483647[ 2147483647] == 2147483647[ 2147483647] == true (Good, true)
-2147483648[ 2147483648] == 2147483648[ 2147483648] == false (Fail, true)
-2147483648[-2147483648] == 2147483648[ 2147483648] == false (Good, false)
-2147483647[ 2147483649] == 2147483649[ 2147483649] == false (Fail, true)
-2147483646[ 2147483650] == 2147483650[ 2147483650] == false (Fail, true)
/* This is a scratch program to demonstrate int vs size_t (unsigned int).
It is not an example of good programming practice. */
#include <stdio.h>
#define TO_STRING(x) #x
#define COMPARE(i,op,u,e) printf("%11d[%11s] %2s %11u[%11s] == %5s (%s, %5s)\n",\
(int)(i),\
#i,\
TO_STRING(op),\
(size_t)(u),\
#u,\
(int(i) op size_t(u))?"true":"false",\
((int(i) op size_t(u)) == e)?"Good":"Fail",\
#e)
#define EQ ==
#define LT <
#define GT >
int main(int, char**) {
printf("The size of int is %d\n", sizeof(int));
printf("The size of size_t (unsigned int) is %d\n", sizeof(size_t));
printf("** The number in [] is the value that is written in the code.\n");
COMPARE(1,EQ,1,true);
COMPARE(1,GT,1,false);
COMPARE(1,LT,1,false);
COMPARE(2,GT,1,true);
COMPARE(-1,LT,1,true);
COMPARE(-1,GT,1,false);
COMPARE(1,LT,-1,false);
COMPARE(1,GT,-1,true);
COMPARE(1,EQ,-1,false);
COMPARE(2147483646,EQ,2147483646,true);
COMPARE(2147483647,EQ,2147483647,true);
COMPARE(2147483648,EQ,2147483648,true);
COMPARE(-2147483648,EQ,2147483648,false);
COMPARE(2147483649,EQ,2147483649,true);
COMPARE(2147483650,EQ,2147483650,true);
return 0;
}
The other thing I just noticed, which is a bit more advanced topic, is that Bots is a std::vector which means is a a container class, which means it has an iterator interface, which quite often results in better performance. It would look something like this:
//A robot?
for (std::vector<Robot>::const_iterator iter = Bots.begin();
iter != Bots.end(); iter++)
{
if ( (*iter).isHere(i,j) )
{
drawthis = '+';
}
}
Though for that to compile we need to change the prototype (and the definition) of Thing::isHere(int,int) so that it is a const function. That is, it promises that it will not change the object that it is acting upon.
bool isHere (int isx, int isy) const;
This is also a slightly more advanced topic, but one that FSO could learn a lot from. FSO does not use const nearly enough for whatever reason (though admittedly because FSO uses so much global data anyway it is a fairly moot point because const cannot protect that data because someone needs to be able modify it). A large number of our bugs stem from functions that seemingly should not need to change anything, but do.
Strictly main also has two more arguments (int argc, char** argv).
Are these the command-line arguments? Should I include support for them even if they're not used anywhere?
Yes, they are the command line arguments. It doesn't hurt to include them because you will have to accept command-line arguments at some point. Obviously it is up to you, but some compilers will always complain about it being missing, others will complain when you set them to pedantic mode.
Because pedantic mode will also complain that you declared them but don't using them, you can declare main as:
int main(int, char**)or you can define a macro that does nothing so that you can name the args but the compiler never sees the name declared. The end result is the compiler seeing the same thing as the main prototype above, but we as coders get the benefit of knowing what each parameter should be used for. Obviously this is a bit overkill for main, and is more helpful when you are talking about functions that not everyone knows the names of the parameters are or for, like in a library or in large code base like FSO. A good example of this would be a flags argument that is part of the standard prototype but not every implementation needs or uses the flags argument.
/* Used to flag a function arg as not used so that
the compiler does not whine about a variable that
is declare but unused. */
#define UNUSED(x)
int main(int UNUSED(argc), char** UNUSED(argv))
-
Alright, that's quite some useful information. I'd been wondering about iterators... Thanks :)
-
Think I'm pretty much done on Robots. Added a save/load function, a legend and some further tweaks. It also compiles and runs on non-Windows now (well, Ubuntu, at least).
Now for the next project... hmm...
[attachment deleted by ninja]
-
T-t-t-triple post! Anything in a mod's data\maps directory should only be referenced by POFs and missions (for texture replace), right? And is there a texture replace SEXP yet?
With my OCD for orderliness, I've been working on a program for cleaning up modpacks... Version 0.1 EXE (Compiled under 32-bit WinXP) attached. For now, all it does is list the maps not used by any POF. Tuck it in \data\ and run - it won't change anything in the pack by itself, only analyse the contents and display the result. So should be safe :)
To do:
- Add support for in-mission texture replacement
- Make compatible with Linux
- (Longer term) Extend functionality to unused effects as well
Most importantly, from a code point of view, am I doing it more or less right?
#define _CRT_SECURE_NO_WARNINGS //Else MSVC complains you should use MSVC-only functions
#include <direct.h> // directory things
#include <dirent.h> // more directory things
#include <fstream>
#include <iostream>
#include <string>
#include <vector>
#include "Mapname.hpp"
//no using namespace std, the binary file reading later seems to confuse MSVC
int main(int, char**) {
//Declarations
int quitting; //just so window doesn't get closed immediately
std::vector <std::string> usedmaps; //will contain maps in use, w/o extensions. Doubles are allowed.
std::vector <Mapname> allmaps; //will contain all maps in data/maps, w/o extensions. Doubles allowed.
std::string strentry; //C++-string: directory entry just read
//C strings, because direct, dirent and ifstream work like that
char datadir [FILENAME_MAX]; //directory containing the program, should be /data/
char modeldir [FILENAME_MAX]; //data/models
char mapdir [FILENAME_MAX]; //data/maps
//dirent.h stuff, copied from a tutorial somewhere...
DIR *dir;
struct dirent *entry;
//model reader
std::ifstream curmodel;
int numtex = 0;
int numchar = 0;
char texname_c [FILENAME_MAX]; //texture name, C-style
std::cout << "Unused map finder, v0.1 -- by FSF\n" << std::endl;
////////////////////////////////////////////////////////////////////
//Set up directories
_getcwd(datadir,FILENAME_MAX); //store working directory in datadir
strcpy (modeldir, datadir); //copy datadir to other strings
strcpy (mapdir, datadir); //not using strcpy_s or strcat_s because those are MSVC++ only
strcat (modeldir, "\\models"); //append subdirectory, we'll check if it exists later
strcat (mapdir, "\\maps"); //forward- or backslash system-dependent?
////////////////////////////////////////////////////////////////////////
//Look for model files, add textures in use to usedmaps
_chdir(modeldir); //set working directory
if (dir = opendir(modeldir))
{
while (entry = readdir(dir))
{
if (strcmp (entry -> d_name, ".") != 0 //strcmp returns 0 if strings identical
&& strcmp (entry -> d_name, "..") != 0)
{
//convert to C++-string (yay for using the two types simultaneously -.-)
strentry = entry -> d_name;
//convert to lowercase
int strlength = strentry.size();
for (int i = 0; i < strlength; i++)
strentry[i] = tolower(strentry[i]);
//Only POF files are interesting
if (strentry.find (".pof") != -1)
{
curmodel.open(strentry, std::ifstream::in | std::ifstream::binary);
if (curmodel.is_open())
{
std::cout << "Parsing model " << strentry << std::flush;
//read number of textures
curmodel.seekg(16,std::ios_base::beg);
curmodel.read((char*) &numtex, 4);
if (numtex == 1)
{
std::cout << ": 1 texture" << std::endl;
}
else
{
std::cout << ": " << numtex << " textures" << std::endl;
}
//iterate over textures
curmodel.seekg(20,std::ios_base::beg);
for (int j = 0; j < numtex; j++)
{
//get texture number of characters
curmodel.read((char*) &numchar, 4);
//read texture name
curmodel.read(texname_c, numchar);
texname_c[numchar] = '\0';
//convert to lowercase
for (int i = 0; i <= numchar; i++)
texname_c[i] = tolower(texname_c[i]);
usedmaps.push_back(texname_c);
//std::cout << "- " << texname_c << std::endl;
}
curmodel.close();
}
else
{
std::cout << "Error opening model " << strentry << std::endl;
}
}
}
}
std::cout << std::endl;
closedir(dir);
}
else
{
std::cout << "Unable to open models directory!" << std::endl;
std::cout << "Check if it exists; all-lowercase is preferred." << std::endl;
std::cout << "Press any key and <ENTER> to quit." << std::endl;
std::cin >> quitting;
return 0;
}
/////////////////////////////////////////////////////////////////////
//Read mission files
/////////////////////////////////////////////////////////////////////
//Make inventory of maps
_chdir(mapdir); //set working directory
if (dir = opendir(mapdir))
{
std::cout << "\nInventorizing maps directory..." << std::endl;
while (entry = readdir(dir))
{
if (entry -> d_name[0] != '.')
{
allmaps.push_back(entry -> d_name);
//std::cout << "Found map: " << entry -> d_name << std::endl;
}
}
closedir(dir);
}
else
{
std::cout << "Unable to open maps directory!" << std::endl;
std::cout << "Check if it exists; all-lowercase is preferred." << std::endl;
std::cout << "Press any key and <ENTER> to quit." << std::endl;
std::cin >> quitting;
return 0;
}
//////////////////////////////////////////////////////////////////////
//Compare maps available with maps in use
std::cout << "Unused maps: " << std::endl;
for (std::vector<Mapname>::const_iterator iter1 = allmaps.begin();
iter1 != allmaps.end(); iter1++)
{
bool inUse = false; //whether or not this specific texture from allmaps is in use
for (std::vector<std::string>::const_iterator iter2 = usedmaps.begin();
iter2 != usedmaps.end(); iter2++)
{
if ((*iter1).getWoExt() == (*iter2))
{
inUse = true;
}
}
if (!inUse)
{
//unusedmaps.push_back((*iter1).getWtExt());
std::cout << "- " << (*iter1).getWtExt() << std::endl;
}
}
std::cout << std::endl;
//for pausing purposes
std::cout << "Press any key and <ENTER> to quit." << std::endl;
std::cin >> quitting;
return 0;
}
#ifndef MAPNAME_HPP
#define MAPNAME_HPP
#include <sstream>
#include <string>
class Mapname
{
private:
std::string file_cpp_ext; //full filename
std::string file_cpp_nex; //"core" filename, lowercase, w/o -glow.dds, -normal.png etc.
public:
Mapname();
Mapname(char c_ext_in []); //construct from C-string
Mapname(std::string cpp_ext_in); //construct from C++-string
std::string getWtExt () const {return file_cpp_ext;}
std::string getWoExt () const {return file_cpp_nex;}
//suppose these could be static
std::string trimName (std::string cpp_ext_in) const; //turn full into 'core' filename
std::string paddedInt (int in) const; //turn integer into 4-digit zero-padded string, e.g. 0034
};
#endif //MAPNAME_HPP
#include "Mapname.hpp"
Mapname::Mapname(char c_ext_in []) //construct from C-string
{
file_cpp_ext = c_ext_in;
file_cpp_nex = trimName(file_cpp_ext);
}
Mapname::Mapname(std::string cpp_ext_in) //construct from C++-string
{
file_cpp_ext = cpp_ext_in;
file_cpp_nex = trimName(file_cpp_ext);
}
std::string Mapname::trimName(std::string cpp_ext_in) const //turn full into core filename
{
cpp_ext_in.resize(cpp_ext_in.rfind('.')); //crop off extension
if (cpp_ext_in.rfind("_") != std::string::npos) //crop off possible EFF number, but only start loop if underscore found
{
for (int i = 0; i <= 9999; i++) //loop over possible numbers, don't just crop everything after an underscore!
{
if (cpp_ext_in.rfind("_" + paddedInt(i)) != std::string::npos)
{
cpp_ext_in.resize(cpp_ext_in.rfind("_" + paddedInt(i)));
break;
}
}
}
//turn into lowercase
int strlength = cpp_ext_in.size();
for (int i = 0; i < strlength; i++)
cpp_ext_in[i] = tolower(cpp_ext_in[i]);
//crop off possible specifiers
if (cpp_ext_in.size() >= 5 && cpp_ext_in.rfind("-glow") == cpp_ext_in.size() - 5)
cpp_ext_in.resize(cpp_ext_in.size() - 5);
if (cpp_ext_in.size() >= 7 && cpp_ext_in.rfind("-normal") == cpp_ext_in.size() - 7)
cpp_ext_in.resize(cpp_ext_in.size() - 7);
if (cpp_ext_in.size() >= 6 && cpp_ext_in.rfind("-shine") == cpp_ext_in.size() - 6)
cpp_ext_in.resize(cpp_ext_in.size() - 6);
if (cpp_ext_in.size() >= 6 && cpp_ext_in.rfind("-trans") == cpp_ext_in.size() - 6) //crop off this as last, maps like xxx-trans-glow are allowed
cpp_ext_in.resize(cpp_ext_in.size() - 6);
return cpp_ext_in;
}
std::string Mapname::paddedInt (int in) const //turn integer into 4-digit zero-padded string, e.g. 0034
{
//Largely based on http://www.cplusplus.com/forum/general/15952/
std::stringstream ss;
std::string daString;
int str_length = 0;
//convert number to string
ss << in;
ss >> daString;
//append zeroes
str_length = daString.length();
for (int i = 0; i < 4 - str_length; i++)
{
daString = "0" + daString;
}
return daString;
}
[attachment deleted by a basterd]
-
I only skimmed some things, might give full comments later but here's what I think:
1) Always use C++ strings. Only when you need to, you can call strname.c_str() to convert to a cstring to pass it to something that requires it, but usually it's never worth it to give up all the extra functionality c++ strings give you. Even if you like the array style string crap, you can still dereference a c++ string with the [] operator like cstrings.
2) It would've been good to abstract all OS specific things (or, god forbid, compiler specific things) into its own class to make things easier to port to linux. It might be more work now after the fact, but you should still think about it. It would be easier than maintaining two separate versions.
3) You have some pretty big blocks there. Whatever IDE your using may support hiding the contents of blocks, but you may want to think of putting as much lower level stuff into inlined functions just to make things a bit more readable.
-
Alright, I removed as many C-strings as I could, put all system dependent stuff in system_dependent.hpp (is that more or less how to do it?) and separated some functions to a separate file for readability.
Also added:
- Texture replacement support for ships.tbl, *-shp.tbm and missions initial-status replacement
- Generation of output file if called with command line argument -o or --outfile
- Verbose mode if asked for (-v or --verbose)
- Linux support (tested on Ubuntu 9.10)
Win32 EXE and code attached, feedback still appreciated.
[attachment deleted by a basterd]
-
Hum. My program "has encountered an error and needs to close". I consistently get it at a specific point (the 545th file) during parsing the effects of a specific mod. Other mod folders, be they larger or smaller, work just fine, and so does the same file when placed in another folder. The event viewer error warning doesn't say much either (see below).
Anyone know what could be common causes of this, or how it can be tracked?
Event Type: Error
Event Source: Application Error
Event Category: None
Event ID: 1000
Date: 1/5/2012
Time: 2:42:14 PM
User: N/A
Computer: CNU8183SFP
Description:
Faulting application modpackcleaner.exe, version 0.0.0.0, faulting module modpackcleaner.exe, version 0.0.0.0, fault address 0x0001bbba.
For more information, see Help and Support Center at http://go.microsoft.com/fwlink/events.asp.
Data:
0000: 41 70 70 6c 69 63 61 74 Applicat
0008: 69 6f 6e 20 46 61 69 6c ion Fail
0010: 75 72 65 20 20 6d 6f 64 ure mod
0018: 70 61 63 6b 63 6c 65 61 packclea
0020: 6e 65 72 2e 65 78 65 20 ner.exe
0028: 30 2e 30 2e 30 2e 30 20 0.0.0.0
0030: 69 6e 20 6d 6f 64 70 61 in modpa
0038: 63 6b 63 6c 65 61 6e 65 ckcleane
0040: 72 2e 65 78 65 20 30 2e r.exe 0.
0048: 30 2e 30 2e 30 20 61 74 0.0.0 at
0050: 20 6f 66 66 73 65 74 20 offset
0058: 30 30 30 31 62 62 62 61 0001bbba
0060: 0d 0a ..
-
run it through VS2010's debugger? its awesome when it comes to stuff like that, as i learned with my vb .net "programming" classes.
i have 0 experience with C++, but i have a hunch it might be a null value somewhere messing it up?
also, for anyone well versed in C++, what is the equivalent of the Try ... Catch ... Else function?
-
Yeah, running it through the debugger is pretty much the only thing that helps here. That error message is sort of useless.
-
also, for anyone well versed in C++, what is the equivalent of the Try ... Catch ... Else function?
I'm not all that well versed, but I do have my C++ book that I go by... ( I just hate the wording in that text, it's almost like I've been married to it, and the honeymoon's far been over )
Luckily, I do have cplusplus.com at hand when I have internet:
http://www.cplusplus.com/doc/tutorial/exceptions/ (http://www.cplusplus.com/doc/tutorial/exceptions/)
The C++ equivalent of the VB Try, Catch, Else statment is the Try, Catch statement with a catch all.
Exceptions are thrown like so:
try
{
if(something_happens)
{
throw my_boomerang;
}
}
They are then caught like so:
// int catcher
catch ( int param )
{
if( param == my_boomerang )
{
throw_it_back();
}
}
// default catch-all
catch (...)
{
tell_user(" You f'd up somewhere. ");
}
-
Hmm, that helped, of course. Turns out my filename class didn't know what to do with no-extension names.
Here's beta 0.3 of the tool; a readme is included. Let's say, GNU GPL v3? (Should probably add that somewhere.)
So what's new?
- Now finds unused maps and effects
- No longer needs to be copied to the mod folder you wanna clean
- No more false positives for hard-coded filenames (or, well, it should - not sure if I got them all)
It should compile and run correctly under Windows and Linux alike, no special dependencies needed.
- Win32 EXE (http://www.mediafire.com/download.php?6a7zfcwln64wddf) (106 kB, from MediaFire)
- Source code (http://www.mediafire.com/download.php?v5g7p2zr95d23op) (12 kB, from MediaFire)
Next step, I'm planning to learn wxWidgets and make a proper UI for this.
EDIT: Extra known issue: weapons.tbl: $BeamInfo:+PAni not recognized
-
Beta Alpha 0.4, now with one more fixed bug and a user interface (should make it easier to use for the masses). Due to wx, performance has taken a minor hit; I'm afraid this will remain until I find a good profiler. I'm also still figuring out how to compile with wx under Linux, so no makefile is provided with this release. If anyone can point me to a good howto (the ones I found so far are rather cryptic), that'd be most appreciated.
Win32 EXE (http://www.mediafire.com/?q5do7qvoy75n7gq) (1.25MB from MediaFire)
Source code (http://www.mediafire.com/?gx6wtuggauuo0lp) (12.5kB from MediaFire)
EDIT: I figured out how to make the Linux binaries, but an assert popped up. Something with wxString::rfind(), by the looks of it. I'll get it fixed in the next version.
-
Beta 0.5, now actually working on non-Windows systems as well. To compile under them, run autogen.sh and make. (Make sure you have wxWidgets installed well, and aclocal, autoconf, automake and all that jazz.) All further info should be in the readme.
Win32 EXE (http://www.mediafire.com/?dpjf6lnrsrq1trl) (1.25MB from MediaFire)
Source code (http://www.mediafire.com/?r7ielp2vtx3vrgf) (15 kB from MediaFire)
Anyone find any issues? Else Imma post this to the FS Tools board some time.
-
:bump: Epilogue: this newbie has just landed a job in (C++) software development. Thanks a lot for getting me started, guys!
-
Mad props to you, mate :)