HELP!!! Dynamic Memory

[seanjintix]

I'm Writing a basic RPG Engine and i'm reading data from a .txt file and taking that data to create different dynamic objects based on what i read from the file. I'm trying to write it so that i just call to the function Load_Resources(obj_type,filename); and then it will create multiple dynamic object of obj_type. The number of objects is unknown until after the file has been read. I have it all setup to create the dynamic objects, but if i try to access the different objects outside of the function it only accesses the last one assigned. Although it assigned that data to object[0] instead of the last object object[1]. How can i set this up to create multiple dynamic objects and have access to all of them outside of the function.

ps: sorry for the poor format. im new to this forum and cant seem to figure out how to include code

//Main Function
int main()
{
   //Equipment class isn't relevant to current problem
   //multiple overload functions based on obj_type
   //Here is just one example
   //in this case obj_type = Equipment
   Equipment *equip;
   Load_Resources(equip,filename);

   //Here's where the problem occurs
   //only equip[0] has a name but its name is what object[1] should be
   std::cout << equip[0].Name << endl << equip[1].Name << endl;
}

//Load_Resource Function
//Opens .txt file and searches for the number of objects for current type
//creates dynamic objects based on number of objects in .txt file
//cycles through each dynamic object and initializes variables
//cycles to next obj

void Load_Resources(Equipment *&obj, filename)
{
   int i = 0;
   int cnt = 0;
   int a;
   string line;
   ifstream Resources(filename);

   //Determines Number of dynamic objects
   if(Resources.is_open())
    {
        while(getline(Resources,line))
            if(line == "Equipment:") ++cnt;
        Resources.close();
    }

    obj = new Equipment[cnt];

    Resources.open(filename);
    if(Resources.is_open())
    {
        //Searches every line of "Resources.txt"
        while(getline(Resources,line))
            {
                //Checks for specific item type
                if(line == "Equipment:")
                {
                    for(;i < cnt; ++i)
                    {
                        //sets variables to current dynamic object
                        getline(Resources,line);
                        obj[i].Name = line;

                        getline(Resources,line);
                        obj[i].Description = line;

                        Resources >> a;
                        obj[i].Durability = a;
                        //exits for loop so that it can switch to the next set 
                        //of data
                        //Prevents assigning the same data to all objects
                        break;
                    }
                }
            }
    }
}

//.txt file
Equipment:
Obsidian Helmet
A Rare Helmet Made of a Really Durable Obsidian.
100

Useable:
Potion
Heals 30% of Players Health
true
.30

Equipment:
Iron Sword
A basic Sword Made From Common Metal.
56

[Chervil]

On the second pass through the file, the for-loop looks a bit unnecessary. Each time the word "equipment" is found, there is just one set of data, so a for-loop seems to be doing too much.

Try this for the second pass:

    Resources.open(filename);
    if (Resources.is_open())
    {
        //Searches every line of "Resources.txt"
        while (getline(Resources,line) && i < cnt)
        {
            //Checks for specific item type
            if (line == "Equipment:")
            {
                //sets variables to current dynamic object
                getline(Resources,line);
                obj[i].Name = line;

                getline(Resources,line);
                obj[i].Description = line;

                Resources >> a;
                obj[i].Durability = a;

                i++;
            }
        }
    }

Having said that, you could make things much simpler by using a vector to handle the dynamic allocation for you.

int main()
{
   vector<Equipment> equip;
   Load_Resources(equip, filename);

   for (unsigned int i=0; i<equip.size(); i++)
       std::cout << equip[i].Name << endl;
}

void Load_Resources(vector<Equipment> &obj,  std::string filename)
{
    ifstream Resources(filename);
    if (!Resources)
    {
        std::cout << "File not open: " << filename << endl;
        return;
    }

    string line;
    while (getline(Resources,line))
    {
        if (line == "Equipment:")
        {
            Equipment item;
            getline(Resources, item.Name );
            getline(Resources, item.Description);
            Resources >> item.Durability;
            obj.push_back(item);
        }
    }
}

[seanjintix]

alright, thanks. ill try that. two questions though.
1: how do i include the code like that?
2: how does the vector part work, i haven't really looked into vectors yet, how are they useful?

[Chervil]

1: how do i include the code like that?

Just add one line:

#include <vector> 

2: how does the vector part work, i haven't really looked into vectors yet, how are they useful?

Lots of ways.
• They reduce the risk of doing something dangerous by manually using new [] and delete [] with raw pointers.

• Automatically resize as and when required, yet are just as easy and fast to use as an plain array.

• Are convenient when you don't know in advance how much data there will be. In this case you read the file twice. But if the user was entering data at the keyboard you could hardly ask them to enter it all and then go back to the beginning and enter it all over again!

• Unlike an array, which when passed as a function parameter doesn't keep track of its size, the vector always keeps track of how many elements it contains.

Start now, you'd never look back :)

[seanjintix]

so basically there pretty close to the same thing except vectors are less prone to errors, right?
what does the "obj.push_back(item);" do?

[Chervil]

The vector at the start has a size of zero (unless deliberately specified otherwise).
The push_back() increases the size by one and then stores the item in that newly available position.
See http://www.cplusplus.com/reference/vector/vector/push_back/

The opposite is also possible, pop_back() removes the last element and reduces the size by 1.

It's worth skimming through the list of available member functions, you may not need most them at first, but sooner or later could come in handy.
http://www.cplusplus.com/reference/vector/vector/

[seanjintix]

Much thanks to you. I have been working on this engine for a little over a week. just recently started programming again. i figured that a big project like this could widen my understanding of programming and possibly get me closer to a more advanced level. i had searched the internet all day for a solution and the answer ended up being so simple. it seems that there is a lot more to understand than i thought. thanks for the help though it simplified my code tremendously.

[seanjintix]

One last question. How would i make a global variable in my Resources.cpp file so that i wont have to create a bunch of overload functions for each type. Instead of having to do:

void Load_Resources(type1,filename);
void Load_Resources(type2,filename);
void Load_Resources(type3,filename);
void Load_Resources(type4,filename);

how would i create a global variable so that it's accessible in the main.cpp similar to

if(line == "type1:")
Global type1.push_back(item);

if(line == "type2:")
Global type2.push_back(item);

Or is there another way to do this? It would help to shorten my code even further

[Chervil]

I'm always wary of global variables, it's occasionally useful, but more often it makes the code harder to follow as it isn't clear which function should be using which variable, and for what purpose.

So as an alternative, perhaps adding additional parameters when calling the function which reads the file.

For example, this function might look a bit like this, (in outline at least)

void Load_Resources(vector<Equipment> &equip, vector<Useable> &use, std::string filename)
{
    ifstream Resources(filename);
    if (!Resources)
    {
        std::cout << "File not open: " << filename << endl;
        return;
    }

    string line;
    while (getline(Resources,line))
    {
        if (line == "Equipment:")
        {
            Equipment item;
            getline(Resources, item.Name );
            getline(Resources, item.Description);
            Resources >> item.Durability;
            equip.push_back(item);
        }
        else if (line == "Useable::")
        {
            Useable item;
            // read from file into item
            use.push_back(item);
        }
        // else if (line ==  etc. )
        //{
        //
        //}
    }
}

I'm not entirely happy with this idea. Though whenever code starts to either look repetitive, or to grow longer than a few lines in a function, it is usually time to break things into smaller functions. This helps to maintain readability, and isolates related actions within their own block of code.

For example, the code to read all the data for an Equipment object could be delegated to a separate function, and similarly with the other types of object. I'm just playing with ideas here.

[seanjintix]

So more or less it would be a lot easier to read if i left each obj type in seperate functions. I guess that makes sense

[Chervil]

I guess you might be right. What I was trying to avoid was having to read the input file multiple times. Though that hardly matters as it would be cached anyway.

My initial thought was to read through the input file and call a corresponding function depending on the type of data identified, passing the input file by reference. But that still means a lot of parameters being passed, which isn't a big deal, but it doesn't scale very well if there are lots of different object types.

Another idea is to place all of the vectors in a container class, thus only a single parameter need be passed to the load resources function.

class allResources {
   vector<Equipment> equip;
   vector<Useable> use;    
};

I'm sure there are lots of other approaches too.

[seanjintix]

shouldn't i use a struct instead of a class in that case though? something similar to

struct Object
{
     vector<Equipment> equip;
     vector<useable> use;
}type;

Load_Resources(type.equip,filename);

or am i thinking of the wrong thing?

[Chervil]

First, let me say I'm not sure it was a particularly good idea which I had in the first place. So what I'm about to say is by way of clarification, but don't take it as a recommended plan.

In C++ struct and class are almost the same thing. The only difference is that by default members of a struct are public, while the default for class is private. For historical reasons, struct tends to be used for data-only purposes, but it doesn't have to.

Myself, I tend to just use class for most purposes, because even those cases which may start out as plain old data often seem to lend themselves to the addition of member functions such as a constructor, or a friend function to print the contents, and so on.

So in that respect, my example was incomplete, it was missing the public keyword:

class allResources {
public:
    vector<Equipment> equip;
    vector<Useable> use;    
};

and your struct then becomes equivalent.

However, I wasn't thinking quite like this:

struct Object
{
     vector<Equipment> equip;
     vector<useable> use;
}type;

Load_Resources(type.equip,filename);

What I had in mind was more like this:

struct allResources 
{
     vector<Equipment> equip;
     vector<useable> use;
};

int main()
{

    allResources  allres;
    Load_Resources(allres, filename);

The struct is declared in the global space. That just tells the compiler what it looks like. Then inside main(), an actual object of type allResources is defined with the name allres. Function Load_Resources will have the ability to access any of the member variables, in order to populate each of the vectors according to the contents of the file.

Other parts of the program (I don't know, this is your program, not mine) might have a need to access just one of the vectors, in which case it could be referred to as allres.equip and so on.

Hope that at least clarifies what I was thinking. But I don't necessarily recommend it. I had other ideas on parsing the input file which went in a completely different direction and may have been better.

[seanjintix]

That would actually simplify my code tremendously though. You say it isn't recommended, is there a more efficient way i could include all my types so that i can use less functions? sorry for all the questions. i'm trying to figure out the best way to make my engine so that it's easier to use for the user.

Edit: I'm also trying to learn and understand as much as possible

[Chervil]

Let me clarify. I only said not recommended simply because I wanted to have a hypothetical discussion without trying to apply it to any specific purpose. I suppose equally I wouldn't discourage it.

I wanted to leave the field open for further exploration of ideas, rather than latching onto the first which comes along.

is there a more efficient way i could include all my types so that i can use less functions?

I'm not sure. If you wanted, you could put all of your code inside of main(). That would use less functions.

But seriously, generally I'd encourage the use of more functions rather than less,

Where it becomes an issue is when more or less the same work is needlessly replicated.

A recent example in this thread:
http://www.cplusplus.com/forum/beginner/181486/#msg890406

[seanjintix]

So more functions is good as long as it helps to improve the program, and as long as it's not to redundant.

[Chervil]

More or less.

When I look at code which appears repetitive, it causes me to question. Repetition is something which humans are bad at while computers excel (we make mistakes, or we get bored). Even if the code is correct, reading it and understanding it, and especially maintaining it can be an issue. So I start to look for ways to reduce repetition, Sometimes that is through the use of arrays or loops, other times placing repeated code in a generalised form in a separate function is the way to go.

But it must be recognised that sometimes seemingly repetitive code might be necessary, and is the best solution.

[seanjintix]

Thanks!! Seriously!! I'm self taught and its helpful in learning to get other perspectives.