Sign in to follow this  
KiLLeR13iii

[SOLVED]Atempt to recreate Extremely Simple Scripting Engine

Recommended Posts

I`m having trouble recreating the code in article http://www.gamedev.net/reference/articles/article1945.asp and adapt it to my needs The code compiles perfectly, but the script does not work. I discovered the problem being here:
tokenize( const std::string &codeVec )
      {
      size_t equalAt = 0,
             commaAt = 0,
             commentAt = 0;

      std::vector<std::string> retString ;

      equalAt = codeVec.find("=");
      commaAt = codeVec.find( "," );
      commentAt = codeVec.find( "%" );


When debugging the codeVec looks like: codeVec "print=100,asd" in the Watch window, but when equalAt,commaAt and commentAt are calculated they all return 4294967295 instead of 5,9,a value greater than codeVec.size(); This is giving me a headache. I can`t understand why the find function won`t return the good result, even though the codeVec string looks OK. Thnx for the help btw. [Edited by - KiLLeR13iii on June 6, 2009 5:02:09 PM]

Share this post


Link to post
Share on other sites
As requested:

br_sl.h

#include <stdlib.h>
#include <set>
#include <vector>
#include <fstream>
#include <ios>
#include <iostream>
#include <wchar.h>
#include <string>
#include "br_parser.h"
using namespace std;



class BR_SL
{
private:
int count; //retine nr de tokens din fisier
BR_Parser parser;
void handleTokens( const std::vector<std::string> &instruction);

public:
BR_SL();
~BR_SL();

int execute( char* filename );
};




br_sl.cpp

#include "br_sl.h"
void BR_SL::handleTokens( const std::vector<std::string> &instruction)
{
if (instruction[0]=="print")
printf("%s,%s",instruction[1].c_str(),instruction[2].c_str);;
//switch((char*)instruction[0].data)
//{
// "print":
// printf("%s,%s",instruction[1],instruction[2]);
//break;
//}

}

BR_SL::BR_SL()
{

}

int BR_SL::execute( char* filename )
{
fstream scriptFile;
string buffer;
std::vector<std::string> tokenizedStr;
// get a filestream and open the file
scriptFile.open(filename);

// read, tokenize and execute each line of the file
while( !scriptFile.eof() )
{
scriptFile.getline((char*)&buffer[0],255,wcout.widen('\n'));
tokenizedStr = parser.tokenize( buffer );
handleTokens( tokenizedStr );

// Clears the tokenizedStr for the next element.
tokenizedStr.clear();
count++;
}

return 0 ;
};




br_parser.h

#include <stdlib.h>
#include <set>
#include <vector>
#include <stdlib.h>
#include <set>
#include <vector>
#include <wchar.h>
class BR_Parser
{
public:
BR_Parser(){};
std::vector<std::string> tokenize( const std::string &codeVec );
};




br_parser.cpp

#include "br_parser.h"
#include <stdlib.h>
#include <set>
#include <vector>
#include <fstream>
#include <ios>
#include <iostream>
#include <wchar.h>
#include <string>
std::vector<std::string> BR_Parser::tokenize( const std::string &codeVec )
{
size_t
equalAt=0,
commaAt=0,
commentAt=0;

std::vector<std::string> retString ;

equalAt = codeVec.find("=");
commaAt = codeVec.find( "," );
commentAt = codeVec.find( "%" );

if( commentAt > codeVec.size() )
{
retString.push_back( codeVec.substr( 0, equalAt ) );
}
else
{
return( retString );
}

if ( equalAt < codeVec.size() )
{
if ( commaAt < codeVec.size() )
{
retString.push_back(codeVec.substr( equalAt+1,
((commaAt)-(equalAt+1)) ) );
retString.push_back( codeVec.substr( commaAt+1,
codeVec.size() ) );
}
else
{
retString.push_back( codeVec.substr( equalAt+1,
codeVec.size() ) );
}
}

return( retString );
};




source.cpp

#include "br_sl.h"
void main()
{
BR_SL *sl;
sl=new BR_SL();
sl->execute("a.txt");

}




here`s the whole source code
the a.txt looks like

print=100,asd

[Edited by - KiLLeR13iii on June 6, 2009 8:50:33 AM]

Share this post


Link to post
Share on other sites
I quickly wrapped up your example in a small program:
#include <iostream>
#include <string>
#include <vector>
#include <cstdlib>

void tokenize( const std::string &codeVec )
{
size_t equalAt = 0,
commaAt = 0,
commentAt = 0;

std::vector<std::string> retString ;

equalAt = codeVec.find("=");
commaAt = codeVec.find( "," );
commentAt = codeVec.find( "%" );

// equalAt = 5
// commaAt = 9
// commentAt = 4294967295 = std::string:npos
}

int main()
{
tokenize("print=100,asd");
}


The reason you're getting that large number is because it equals to std::string::npos when the searched string isn't found.
I compiled this under Visual C++ Express 2008.

Share this post


Link to post
Share on other sites
The issue is inside your br_sl.cpp code.

You declare "string buffer;" which would create an empty string. You then cast the buffer to a char* when using getline. You're essentially overwriting the strings' internal memory (and possibly other stack data).

I created a 256 character read buffer (since that appeared to be the length you used), and then filled the string with the data after it has been read:


int BR_SL::execute( char* filename )
{
fstream scriptFile;
string buffer;
char readBuffer[256];
std::vector<std::string> tokenizedStr;
// get a filestream and open the file
scriptFile.open(filename);

// read, tokenize and execute each line of the file
while( !scriptFile.eof() )
{
memset(readBuffer, 0, 256);
scriptFile.getline(readBuffer,255,wcout.widen('\n'));
buffer = readBuffer;
tokenizedStr = parser.tokenize( buffer );
handleTokens( tokenizedStr );

// Clears the tokenizedStr for the next element.
tokenizedStr.clear();
count++;
}

return 0 ;
};

Share this post


Link to post
Share on other sites
The positions look OK now .. i`m always having a trouble with memory, i`m not that much thinking into how it really behaves.. Thnx for your time and help.

Share this post


Link to post
Share on other sites
You are making life more difficult for yourself. When using the vector, string and fstream objects you shouldn't need to use dynamic allocation.

Main can be written like this:

// main() returns "int" in standard C++
int main()
{
// don't use "new" when it is unnecessary.
BR_SL sl;
sl.execute("a.txt");

}


Your classes names are too dense to have meaning. BR appears to be a project prefix (which you can use namespaces for). SL stands for... scripting language? Be more explicit with the names.

You can remove the constructor and destructor from BR_SL, the compiler will generate the same code. You should only add a constructor or destructor if it serves a purpose. Also, keep in mind the rule of three.

You can make better use of fstream and string:

// Don't return a constant value.
// Especially if you aren't going to check it in main().
void BR_SL::execute( char* filename )
{
// you can open the file using its constructor
// rather than the open() member function
fstream file(filename);
string line;

// the free function std::getline(std::istream &, std::string &)
// is preferred to messing with arrays of some arbitrary max size
while( std::getline(file, line )
{
// declare locals as close to use as possible
// by declaring the vector here, we remove the need
// to clear() it explicitly
std::vector<std::string> tokens = parser.tokenize( buffer );
handleTokens( tokens );
count++;
}
} // you don't need a semicolon following a function body



This next part has a number of bugs:

void BR_SL::handleTokens( const std::vector<std::string> &instruction)
{
if ((char*)instruction[0].data()=="print")
printf("%s,%s",instruction[1],instruction[2]);;
}


You are comparing two pointers for equality. Much better to use std::string::operator==, which is cleaner and does what you expect. Also, you are sending raw std::string instances to printf, which is incorrect. As a C function, printf only understands certain types, and expects %s to be matched by a const char * in the argument list. You can use std::cout instead, it is type safe. You should check that the vector isn't empty before checking for a command, and also that you have enough arguments for the command:

void BR_SL::handleTokens( const std::vector<std::string> &instruction)
{
if(instruction.empty())
{
// error handling, or just ignore the empty line
}
else if (instruction.front() == "print")
{
// print expects 2 additional parameters.
if(instruction.size() != 3)
{
// too few or too many parameters
// add error handling here
}
else
{
std::cout << instruction[1] << ',' << instruction[2];
}
}
}

Share this post


Link to post
Share on other sites
Thank you for your suggestions. Actually that was the first time i used std .. as stated, i attempted to recreate the Extremely Simple Scripting Engine
by Cyberdrek, but somewhere along the way i messed up something and tried to fix it with my knowledge. And the result was seen :)) Anyway i`m good now .. and i`ll modify as u said .. looks more robust. That error i noticed too and fixed right after sorting with the actual problem.About the dynamic allocation i`m intending to extend this to suit the needs of a 3D engine(models,maps,players,buttons,etc).
Anyway .. i know that i still suck at writing expert code, but i`m trying to get there, and accepting criticism is the first step.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this