Author Topic: Criticize a newbie  (Read 6516 times)

0 Members and 1 Guest are viewing this topic.

Offline Iss Mneur

  • 210
  • TODO:
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)
Code: [Select]
cl /EHsc /out:robots.exe robots.cpp things.cpp main.cpp

Code: (robots.hpp) [Select]
#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_
Code: (robots.cpp) [Select]
#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); }
}
}
Code: (things.cpp) [Select]
#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)
//{

//}
Code: (things.hpp) [Select]
#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
Code: (main.cpp) [Select]
#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.
"I love deadlines. I like the whooshing sound they make as they fly by." -Douglas Adams
wxLauncher 0.9.4 public beta (now with no config file editing for FRED) | wxLauncher 2.0 Request for Comments

 
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.

Quote
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?

 

Offline Iss Mneur

  • 210
  • TODO:
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.
Code: (robots.cpp) [Select]
//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.

Code: (in 32 bit code with VS2010) [Select]
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)
Code: (in 64 bit code (mismatch size) with VS2010) [Select]
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)
Code: (signed_unsigned.cpp) [Select]
/* 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:
Code: (Loop using an iterator) [Select]
//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.
Code: (things.hpp) [Select]
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.

Quote
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:
Code: (Main prototype without variable names) [Select]
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.
Code: (Some header somewhere) [Select]
/* 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)
Code: (Main prototype with helpful names for coders) [Select]
int main(int UNUSED(argc), char** UNUSED(argv))
"I love deadlines. I like the whooshing sound they make as they fly by." -Douglas Adams
wxLauncher 0.9.4 public beta (now with no config file editing for FRED) | wxLauncher 2.0 Request for Comments

 
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?

Code: (POFReader.cpp) [Select]
#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;
}

Code: (Mapname.hpp) [Select]
#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

Code: (Mapname.cpp) [Select]
#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]
« Last Edit: October 30, 2011, 03:18:50 pm by FreeSpaceFreak »

 

Offline Polpolion

  • The sizzle, it thinks!
  • 211
    • Minecraft
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?

Code: [Select]
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                     ..     

 

Offline pecenipicek

  • Roast Chicken
  • 211
  • Powered by copious amounts of coffee and nicotine
    • Minecraft
    • Skype
    • Steam
    • Twitter
    • PeceniPicek's own deviantart page
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?
Skype: vrganjko
Ho, ho, ho, to the bottle I go
to heal my heart and drown my woe!
Rain may fall and wind may blow,
and many miles be still to go,
but under a tall tree I will lie!

The Apocalypse Project needs YOU! - recruiting info thread.

 

Offline The E

  • He's Ebeneezer Goode
  • Global Moderator
  • 213
  • Nothing personal, just tech support.
    • Skype
    • Steam
    • Twitter
Yeah, running it through the debugger is pretty much the only thing that helps here. That error message is sort of useless.
**** every cause that ends in murder and children crying. ― Iain Banks
Join the fun at the HLP IRC channel. Get the latest spam and gossip as long as it's fresh!

 

Offline z64555

  • 210
  • Self-proclaimed controls expert
    • Minecraft
    • Steam
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/

The C++ equivalent of the VB Try, Catch, Else statment is the Try, Catch statement with a catch all.

Exceptions are thrown like so:
Code: [Select]
try
{
  if(something_happens)
    {
      throw my_boomerang;
    }
}

They are then caught like so:
Code: [Select]
// int catcher
catch ( int param )
{
  if( param == my_boomerang )
  {
      throw_it_back();
  }
}
// default catch-all
catch (...)
{
   tell_user(" You f'd up somewhere. ");
}

I'm on Facebook! sort of. Zeesixtyfour Fivefiftyfive

-=wxFRED2=-
R.I.P. Oliver
------------
EveningTea: Time to go Freeman on this cultist..
* EveningTea pulls crowbar off his shoulderstrap and charges screaming incoherently across the marsh *
------------
z64555: bro. do you even salad
------------
z64555: suprise double quaternion!

 
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 (106 kB, from MediaFire)
- Source code (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
« Last Edit: January 24, 2012, 01:51:11 pm by FreeSpaceFreak »

 
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 (1.25MB from MediaFire)
Source code (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.
« Last Edit: February 06, 2012, 11:26:18 am by FreeSpaceFreak »

 
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 (1.25MB from MediaFire)
Source code (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!

 

Offline pecenipicek

  • Roast Chicken
  • 211
  • Powered by copious amounts of coffee and nicotine
    • Minecraft
    • Skype
    • Steam
    • Twitter
    • PeceniPicek's own deviantart page
Mad props to you, mate :)
Skype: vrganjko
Ho, ho, ho, to the bottle I go
to heal my heart and drown my woe!
Rain may fall and wind may blow,
and many miles be still to go,
but under a tall tree I will lie!

The Apocalypse Project needs YOU! - recruiting info thread.