quote:Original post by jamessharpe
Just a small point, that your code is leaking, if the if expression is false, then tempweapon is never deleted, so you should move the delete tempweapon to outside the if statement.
i wouldnt even call new without knowing if you actually need it. so i'd rather place that inside the if instead of creating and deleting the weapon a hundred times and only use it once.
but then.. i wouldnt even initialize it. drop the tmpweapon and call the newweapon constructor in each case, instead of assigning all but one value to a tmp weapon. or drop the newweapon and return the tmpweapon (which is in fact pretty much the same).
all this seems kind of redundant and not necessary.
about the time: whereever you check for the elapsed time, place a static float TimeUntilSpawn (inside it representing the time since a weapon spawned (or whatever event triggers those 3 seconds). as youre most likely testing that every frame it should like this:
static float TimeUnitlSpawn=0; //spawn at first time
if (TimeUntilSpawn -= ElapsedTime <= 0)
{
SpawnWeapon();
TimeUntilSpawn=WEAPON_SPAWN_DELAY;
}
ElapsedTime would be the time since your last frame, that i would calculate once at the beginning of the game loop and pass it to all functions that are time dependent (physics, ai, whatever).
while im at it:
void SpawnWeapon() { int randWeapon = 1 + (rand()%5); switch(randWeapon) { case 1: VecWeapons.push_back(new Weapon(BURST, 3, AMMO_BURST)); break; case 2: VecWeapons.push_back(new Weapon(LASER, 8, AMMO_LASER)); break; ... }}
VecWeapons is considered a vector/array/list or whatever you use to store your weapons in (or a more general container for gameobjects). instead of numWeapons you have VecWeapons.size().
also youre (in your code) obviously drawing the weapon only once when it is created, dont store its pointer anywhere and therefore have another leak (and dont be surprised if your weapon doesnt appear anywhere). your render function shouldnt contain anything but rendering, in this case (i'll use the vector array like instead of stl like):
void Render() { int numWeaps=VecWeapons.size(); //dont know how clever your compiler is, but to be sure it wont call the function every iteration i store it here. for (int i=0; i<numWeaps; ++i) { Weapon* w=VecWeapon[i]; //just me being lazy glTranslatef(w->position.xpos, w->position.ypos, w->position.zpos); VecWeapon[i].Draw(); }}
another thing. consider using a factory. its job would be creating and deleting weapons. why would this be a good idea:
all created weapons are stored within the factory (or call it manager). you want get memory leaks because you were overwriting a pointer and you wont have two pointers to the same object (well, you will, but you wont try to delete them because thats the job of the manager). this could even create weapons of a derived type. im not keen on singletons so im doing something that might look pretty ugly... lots of statics. ill try to do a general factory/manager, not just for guns, but game objects in general.
class ObjectFactory {private: ObjectFactory(); //no instance possible static list<Object*> Objects;public: static Weapon* CreateWeapon(int Type); static Vehicle* CreateVehicle(int Type); static void Destroy(Object* obj); static void CleanUp();};list<Object*> ObjectFactory::Objects;Weapon* ObjectFactory::CreateWeapon(int Type) { Weapon* result=0; switch (Type) { case WEAP_TYPE_LASER: result=new Weapon(bla, bla, bla); //set whatever break; default: cout << "Unknown Weapon Type requested" << endl; } return result;}void ObjectFactory::Destroy(Object* obj) { Objects.remove(obj); delete obj;}void ObjectsFactory::CleanUp() { while (Objects.size()) { delete Objects.front(); Objects.pop_front(); }}
looks like a lot of pointless work, but it never hurts to have everything at one place and not flying around everywhere.
to get a weapon you would do this:
Weapon* weapon=ObjectFactory::CreateWeapon(WEAP_TYPE_LASER);
if you dont need it anymore:
ObjectFactory::Destroy(weapon);
and after a level :
ObjectFactory::CleanUp();
the factory might be extra work, but the rest of the code will be a lot cleaner.
[edited by - Trienco on May 31, 2003 11:51:01 AM][edited by - Trienco on May 31, 2003 11:52:30 AM]
