<?xml version="1.0"?>
<rss version="2.0">
	<channel>
		<title>in need of some opinions</title>
		<link>http://www.allegro.cc/forums/view/587698</link>
		<description>Allegro.cc Forum Thread</description>
		<webMaster>matthew@allegro.cc (Matthew Leverton)</webMaster>
		<lastBuildDate>Sat, 23 Sep 2006 18:33:02 +0000</lastBuildDate>
	</channel>
	<item>
		<description><![CDATA[Hiya. I'm after a second opinion on this particle engine that I've done. It is part of a much larger interface so some of the coding might appear foreign. Especially the Sprite class. Pay no attention to this as the Sprite class has already been debugged, refined, and tested hundreds of times. The particle engine is simple. I've commented the code here so, please have a look and any advice would be appreciated.<br />
<br />
header file:<br />
[code]<br />
#ifndef PARTICLE_H<br />
#define PARTICLE_H<br />
<br />
<br />
#include &lt;allegro.h&gt;<br />
#include &lt;math.h&gt;<br />
#include &quot;Sprites.h&quot;<br />
<br />
<br />
<br />
<br />
#define PARTICLE_OK	AL_ID('P','C','O','K')<br />
#define PARTICLE_MEMORY	AL_ID('P','M','E','M')<br />
#define PARTICLE_TIMER	AL_ID('P','T','I','M')<br />
<br />
<br />
<br />
// particle animation types<br />
#define EXPLOSION		AL_ID('A','E','X','P')	// the particles are forced outward<br />
#define IMPLOSION		AL_ID('A','I','M','P')	// the particles are drawn inward<br />
<br />
// particle primitive types<br />
#define PRIM_LINE		AL_ID('L','I','N','E')	// line<br />
#define PRIM_SPRITE	AL_ID('S','P','R','I')	// animation<br />
#define PRIM_CIRCLE	AL_ID('C','I','R','C')	// circle<br />
<br />
<br />
struct point_t {<br />
	int x, y;<br />
};<br />
<br />
<br />
<br />
<br />
struct Particle {<br />
	point_t position;<br />
<br />
	volatile int time_of_emission;<br />
	volatile int current_time;<br />
<br />
	float speed;<br />
<br />
	float size;<br />
	float bearing;<br />
	float distance;<br />
<br />
	volatile int lifetime;<br />
};<br />
<br />
<br />
<br />
<br />
static volatile int particle_counter = 0;<br />
void ParticleTick(void);<br />
<br />
<br />
<br />
<br />
class ParticleAnimation {<br />
public:<br />
	ParticleAnimation();<br />
	~ParticleAnimation();<br />
<br />
private:<br />
	Particle *ParticleList;<br />
	int nParticles;<br />
<br />
	int c1, c2;		// color 1, color 2<br />
	BITMAP *texture;	// texture (if the effect requires it)<br />
	Sprite *spr;		// sprite (if the effect requires it)<br />
	float falloff;		// fall-off factor for the particles (just a constant float)<br />
<br />
	int duration;			// may be switched off at anytime if equal to -1<br />
	volatile int start_time;	// particle_counter value at the start of this animation<br />
	<br />
	int anim_type;		// Either EXPLOSION or IMPLOSION<br />
	int prim_type;		// either LINE, SPRITE, or CIRCLE<br />
<br />
	point_t position;	// position on the screen<br />
	point_t centre;		// centre on the output bitmap<br />
	float maxSize;		// max size of the particle<br />
	float maxDistance;	// max distance of the particle from the centre<br />
<br />
	BITMAP *pOut;		// output bitmap<br />
	bool used;		// whether or not this memory space is in use<br />
<br />
public:<br />
	int CreateParticleAnimation(int maxp, int col1, int col2, BITMAP *t, Sprite *s, float f, int d, int at, int pt, int x, int y, float mSize, float mDis);<br />
<br />
	/*<br />
		With particles there are animation types and primitive types:<br />
			Animation types:<br />
				Explosion - the particles move away from the source at a decreasing speed<br />
				Implosion - the particles move towards the source at an increasing speed<br />
			Primitive types:<br />
				Line   - Just a simple line where position is the midpoint of the line. This uses<br />
					     texture for effect, but uses an interpolation between c1 and c2 if texture<br />
					     is equal to null. Length of the line is proportionate to speed, time, and<br />
						 inversely proportionate to maxSize<br />
			    	Sprite - Animates a sprite. The sprite is drawn, taking x and y origin values into<br />
						 account, at position, and uses an interpolation of c1 and c2 for colorization.<br />
						 Size is a relative scale factor between 0.0f and 1.0f.<br />
				Circle - Draws a circle with centre position. Like Line, size is proportionate to speed,<br />
						 time, and inversely proportionate to maxSize. Texture is used for effect, or<br />
						 the c1 value is interpolated from the outside inwards to c2 if texture equals<br />
						 null.<br />
<br />
		 Note: texture and sprite cannot both be used.<br />
	*/<br />
<br />
	// respective primitive type processors<br />
	int ProcessLine(int pIn);<br />
	int ProcessSprite(int pIn);<br />
	int ProcessCircle(int pIn);<br />
    <br />
	// respective animation type processors<br />
	int ProcessExplosion(int pIn);<br />
	int ProcessImplosion(int pIn);<br />
<br />
	// process the animation<br />
	int ProcessAnimation();<br />
<br />
	// render from output bitmap to dest<br />
	void Render(BITMAP *dest);<br />
<br />
	// the screen position<br />
	point_t *GetScreenPosition();<br />
	void SetScreenPosition(point_t *p);<br />
<br />
	// free the memory<br />
	void DestroyParticleAnimation();<br />
<br />
	// new particles<br />
	int AddParticle(float bearing, int eType);<br />
	int RemoveParticle(int pIn);<br />
<br />
	// the memory space used?<br />
	bool InUse() { return used; }<br />
	void IsUsed() { used = true; }<br />
	void UnUsed() { used = false; }<br />
};<br />
<br />
<br />
<br />
<br />
<br />
class ParticleEngine {<br />
public:<br />
	ParticleEngine();<br />
	~ParticleEngine();<br />
<br />
private:<br />
	ParticleAnimation *pAnim;<br />
	int nP;	// number of animations<br />
<br />
public:<br />
	// initialize the engine and allocate space<br />
	int CreateParticleEngine(int nParticleAnimations);<br />
	// Process the engine<br />
	int ProcessParticleEngine();<br />
	// free the memory used by the engine<br />
	void DestroyParticleEngine();<br />
<br />
	int Get_nP() { return nP; }<br />
<br />
	// new animations<br />
	int AddParticleAnimation(ParticleAnimation *pA);<br />
	void RemoveAnimation(int index);<br />
};<br />
<br />
<br />
<br />
<br />
<br />
<br />
#endif<br />
[/code]<br />
<br />
<br />
<br />
<br />
<br />
source file:<br />
[code]<br />
#include &quot;Particle.h&quot;<br />
<br />
<br />
<br />
<br />
void ParticleTick(void) {<br />
	particle_counter++;<br />
}<br />
<br />
END_OF_FUNCTION(ParticleTick)<br />
<br />
<br />
<br />
<br />
<br />
ParticleAnimation::ParticleAnimation() {<br />
	ParticleList = NULL;<br />
	nParticles = 0;<br />
<br />
	c1 = c2 = 0;<br />
	texture = NULL;<br />
	spr = NULL;<br />
<br />
	falloff = 0.0f;<br />
<br />
	duration = -1;<br />
	start_time = -1;<br />
	<br />
	anim_type = -1;<br />
	prim_type = -1;<br />
<br />
	maxSize = -1;<br />
	maxDistance = -1;<br />
<br />
	pOut = NULL;<br />
<br />
	used = false;<br />
}<br />
<br />
<br />
<br />
<br />
ParticleAnimation::~ParticleAnimation() {<br />
	if(spr)<br />
		spr-&gt;DeleteSprite();<br />
<br />
	if(texture)<br />
		destroy_bitmap(texture);<br />
<br />
	if(pOut)<br />
		destroy_bitmap(pOut);<br />
}<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::CreateParticleAnimation(int maxp, int col1, int col2, BITMAP *t, Sprite *s, float f, int d, int at, int pt, int x, int y, float mSize, float mDis) {<br />
	nParticles = maxp;<br />
	ParticleList = (Particle*)malloc(sizeof(Particle)*nParticles);	// allocate memory<br />
	if(!ParticleList)<br />
		return PARTICLE_MEMORY;<br />
<br />
    c1 = col1;<br />
	c2 = col2;<br />
	<br />
	texture = t;<br />
	spr = s;<br />
<br />
	if(at == EXPLOSION)<br />
		falloff = f;	// the particles move away from the origin<br />
	else if(at == IMPLOSION)<br />
		falloff = -f;	// the particles move towards the origin<br />
<br />
	duration = d;<br />
	start_time = particle_counter;<br />
	<br />
	anim_type = at;<br />
	prim_type = pt;<br />
<br />
	maxSize = mSize;<br />
	maxDistance = mDis;<br />
<br />
	position.x = x;<br />
	position.y = y;<br />
<br />
	int width = 2 * ( maxDistance + maxSize );<br />
<br />
	pOut = create_bitmap(width, width);	// allocate memory for the output<br />
	if(!pOut) {<br />
		free(ParticleList);<br />
		return PARTICLE_MEMORY;<br />
	}<br />
<br />
	centre.x = width / 2;<br />
	centre.y = width / 2;<br />
<br />
	clear_bitmap(pOut);<br />
<br />
	return PARTICLE_OK;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
// called for a line-type particle<br />
int ParticleAnimation::ProcessLine(int pIn) {<br />
	point_t p1, p2;<br />
<br />
	// the position of the particle is midpoint of the line. therefore:<br />
<br />
	// Miscellaneous #1<br />
	p1.x = ParticleList[pIn].position.x - ( ( ParticleList[pIn].size * cosf( ParticleList[pIn].bearing ) ) / 2 );<br />
	p1.y = ParticleList[pIn].position.y - ( ( ParticleList[pIn].size * sinf( ParticleList[pIn].bearing ) ) / 2 );<br />
<br />
	p2.x = ParticleList[pIn].position.x + ( ( ParticleList[pIn].size * cosf( ParticleList[pIn].bearing ) ) / 2 );<br />
	p2.y = ParticleList[pIn].position.y + ( ( ParticleList[pIn].size * sinf( ParticleList[pIn].bearing ) ) / 2 );<br />
<br />
	// set the drawing mode<br />
	if(texture != NULL) {<br />
		drawing_mode(DRAW_MODE_COPY_PATTERN, texture, 0, 0);<br />
	} else {<br />
		drawing_mode(DRAW_MODE_SOLID, NULL, 0, 0);<br />
	}<br />
<br />
	int b = p2.y - p1.y;<br />
	int x, y, c, d;<br />
<br />
	for(y = p1.y; y != p2.y+b; y++) {<br />
		x = ( ( ( p2.x - p1.x ) * ( y - p1.y ) ) / ( p2.y - p1.y ) ) + p1.x;	// two-point formula for a line (miscellaneous #3)<br />
		d = sqrtf( ( x - p1.x ) * ( x - p1.x ) + ( y - p1.y ) * ( y - p1.y ) );	// get the distance of current point from start<br />
		c = c1 + ( ( c2 - c1 ) * ( ( ftofix(ParticleList[pIn].size) - d ) / ftofix(ParticleList[pIn].size) ) ); // interpolate color based on distance of current point to end (miscellaneous #2)<br />
<br />
		putpixel( pOut, x, y, c );<br />
	}<br />
<br />
	// reset the drawing mode<br />
	drawing_mode(DRAW_MODE_SOLID, NULL, 0, 0);<br />
<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::ProcessSprite(int pIn) {<br />
	int x = ParticleList[pIn].position.x;<br />
	int y = ParticleList[pIn].position.y;<br />
<br />
	// don't worry about this one<br />
<br />
	spr-&gt;UpdateAnimation();<br />
<br />
	spr-&gt;Render(pOut, x, y, true, 0);<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::ProcessCircle(int pIn) {<br />
	if(texture != NULL) {<br />
		drawing_mode(DRAW_MODE_COPY_PATTERN, texture, 0, 0);<br />
	} else {<br />
		drawing_mode(DRAW_MODE_SOLID, NULL, 0, 0);<br />
	}<br />
<br />
	int c;<br />
	int x = ParticleList[pIn].position.x;<br />
	int y = ParticleList[pIn].position.y;<br />
<br />
	for(int r = ParticleList[pIn].size; r &gt; 0; r--) {<br />
		// miscellaneous #2<br />
		c = c1 + ( ( c2 - c1 ) * ( ( ftofix(ParticleList[pIn].size) - r ) / ftofix(ParticleList[pIn].size) ) );<br />
        circlefill(pOut, x, y, r, c);<br />
	};<br />
<br />
	drawing_mode(DRAW_MODE_SOLID, NULL, 0, 0);<br />
<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::ProcessExplosion(int pIn) {<br />
	int x, y;<br />
	float bearing;<br />
<br />
	bearing = ParticleList[pIn].bearing;<br />
<br />
	x = ParticleList[pIn].position.x + ftofix( ParticleList[pIn].distance * cosf( bearing ) );<br />
	y = ParticleList[pIn].position.y + ftofix( ParticleList[pIn].distance * sinf( bearing ) );<br />
<br />
	ParticleList[pIn].position.x = x;<br />
	ParticleList[pIn].position.y = y;<br />
<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::ProcessImplosion(int pIn) {<br />
	int x, y;<br />
	float bearing;<br />
<br />
	bearing = ParticleList[pIn].bearing;<br />
<br />
	x = ParticleList[pIn].position.x - ftofix( ParticleList[pIn].speed * cosf( bearing ) );<br />
	y = ParticleList[pIn].position.y - ftofix( ParticleList[pIn].speed * sinf( bearing ) );<br />
<br />
	ParticleList[pIn].position.x = x;<br />
	ParticleList[pIn].position.y = y;<br />
<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::ProcessAnimation() {<br />
	volatile int dietime = start_time + duration;<br />
<br />
	if(dietime == particle_counter &amp;&amp; duration != -1) {<br />
		DestroyParticleAnimation();<br />
		return 0;<br />
	}<br />
<br />
	for(int pIn = 0; pIn &lt; nParticles; pIn++) {<br />
		if(ParticleList[pIn].time_of_emission != -1) {<br />
			dietime = ParticleList[pIn].lifetime;<br />
<br />
			ParticleList[pIn].current_time = particle_counter - ParticleList[pIn].time_of_emission;<br />
			if(ParticleList[pIn].current_time &gt; dietime)<br />
				return RemoveParticle(pIn);<br />
<br />
			switch(anim_type) {<br />
				case EXPLOSION:<br />
					// EXP#1<br />
					ParticleList[pIn].speed = falloff * sqrt((float)(ParticleList[pIn].current_time * ParticleList[pIn].speed));<br />
					// EXP#2<br />
					ParticleList[pIn].distance += ParticleList[pIn].speed;<br />
					// EXP#3<br />
					ParticleList[pIn].size = ( falloff * ParticleList[pIn].distance ) / maxSize;<br />
<br />
					ProcessExplosion(pIn);<br />
				break;<br />
				case IMPLOSION:<br />
					// IMP#1<br />
					ParticleList[pIn].speed = ( maxDistance * falloff ) / ( sqrt( 1 + ParticleList[pIn].distance ) );<br />
					// IMP#2<br />
					ParticleList[pIn].distance += ParticleList[pIn].speed;<br />
					// IMP#3<br />
					ParticleList[pIn].size = ( ParticleList[pIn].distance / maxDistance ) * maxSize;<br />
<br />
					ProcessImplosion(pIn);<br />
				break;<br />
			};<br />
<br />
			switch(prim_type) {<br />
				case PRIM_LINE:<br />
					ProcessLine(pIn);<br />
				break;<br />
				case PRIM_SPRITE:<br />
					ProcessSprite(pIn);<br />
				break;<br />
				case PRIM_CIRCLE:<br />
					ProcessCircle(pIn);<br />
				break;<br />
			};<br />
<br />
			srand(particle_counter);<br />
			int angle = rand() % 360;<br />
<br />
			AddParticle((float)((angle*AL_PI)/180), anim_type);<br />
		}<br />
	};<br />
<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
void ParticleAnimation::Render(BITMAP *dest) {<br />
	masked_blit(pOut, dest, 0, 0, position.x, position.y, pOut-&gt;w, pOut-&gt;h);<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
point_t *ParticleAnimation::GetScreenPosition() {<br />
	return &amp;position;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
void ParticleAnimation::SetScreenPosition(point_t *p) {<br />
	position.x = p-&gt;x;<br />
	position.y = p-&gt;y;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
void ParticleAnimation::DestroyParticleAnimation() {<br />
	if(ParticleList)<br />
		free(ParticleList);<br />
<br />
	if(pOut)<br />
		destroy_bitmap(pOut);<br />
<br />
	UnUsed();<br />
}<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::AddParticle(float bearing, int eType) {<br />
	for(int i=0; ParticleList[i].time_of_emission != -1; i++);<br />
<br />
	if(i &gt;= nParticles)<br />
		return -1;<br />
<br />
	ParticleList[i].bearing = bearing;<br />
	ParticleList[i].time_of_emission = particle_counter;<br />
	ParticleList[i].current_time = 0;<br />
<br />
	switch(eType) {<br />
		case EXPLOSION:<br />
			ParticleList[i].distance = 0.0f;<br />
			ParticleList[i].size = 0.0f;<br />
			ParticleList[i].speed = falloff * sqrt( 1.0f );<br />
<br />
			// EXP#4<br />
			ParticleList[i].lifetime = ( maxSize*maxSize ) / ( falloff * ParticleList[i].speed );<br />
		break;<br />
		case IMPLOSION:<br />
			ParticleList[i].distance = maxDistance;<br />
			ParticleList[i].size = maxSize;<br />
			ParticleList[i].speed = ( falloff * maxDistance ) / sqrt( 1.0f + ParticleList[i].distance );<br />
<br />
			// IMP#4<br />
			ParticleList[i].lifetime = maxDistance / ( ParticleList[i].speed * maxSize * maxSize );<br />
		break;<br />
	}<br />
<br />
	int quadrant;<br />
	float t = fabs( (AL_PI/2) - bearing );	// convert to true bearing<br />
<br />
	if(t &gt;= 0 &amp;&amp; t &lt;= AL_PI/2)<br />
		quadrant = 1;<br />
	else if(t &gt; AL_PI/2 &amp;&amp; t &lt;= AL_PI)<br />
		quadrant = 2;<br />
	else if(t &gt; AL_PI &amp;&amp; t &lt;= (3*AL_PI)/2)<br />
		quadrant = 3;<br />
	else if(t &gt; (3*AL_PI)/2 &amp;&amp; t &lt;= 2*AL_PI)<br />
		quadrant = 4;<br />
<br />
	int x, y;<br />
<br />
	x = ParticleList[i].distance * cosf( bearing );<br />
	y = ParticleList[i].distance * sinf( bearing );<br />
<br />
	switch(quadrant) {<br />
		case 1:<br />
			ParticleList[i].position.x = x;<br />
			ParticleList[i].position.y = y;<br />
		break;<br />
		case 2:<br />
			ParticleList[i].position.x = x;<br />
			ParticleList[i].position.y = -y;<br />
		break;<br />
		case 3:<br />
			ParticleList[i].position.x = -x;<br />
			ParticleList[i].position.y = -y;<br />
		break;<br />
		case 4:<br />
			ParticleList[i].position.x = -x;<br />
			ParticleList[i].position.y = y;<br />
		break;<br />
	};<br />
<br />
	return i;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
int ParticleAnimation::RemoveParticle(int pIn) {<br />
	ParticleList[pIn].time_of_emission = -1;<br />
	return pIn;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
<br />
ParticleEngine::ParticleEngine() {<br />
	nP = 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
ParticleEngine::~ParticleEngine() {<br />
<br />
}<br />
<br />
<br />
<br />
<br />
<br />
int ParticleEngine::CreateParticleEngine(int nParticleAnimations) {<br />
	nP = nParticleAnimations;<br />
	pAnim = (ParticleAnimation*)malloc(sizeof(ParticleAnimation) * nP);<br />
	if(!pAnim)<br />
		return PARTICLE_MEMORY;<br />
<br />
	if(install_int_ex(tick, MSEC_TO_TIMER(25)) &lt; 0) {<br />
		return	PARTICLE_TIMER;<br />
	}<br />
<br />
	return PARTICLE_OK;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
int ParticleEngine::ProcessParticleEngine() {<br />
	for(int a=0; a &lt; nP; a++) {<br />
		pAnim[a].ProcessAnimation();<br />
	}<br />
<br />
	return 0;<br />
}<br />
<br />
<br />
<br />
<br />
<br />
void ParticleEngine::DestroyParticleEngine() {<br />
	if(pAnim)<br />
		free(pAnim);<br />
<br />
	nP = 0;<br />
}<br />
<br />
<br />
<br />
<br />
int ParticleEngine::AddParticleAnimation(ParticleAnimation *pA) {<br />
	for(int a=0; pAnim[a].InUse() == true; a++);<br />
<br />
	if(a &gt;= nP)<br />
		return -1;<br />
<br />
	pAnim[a] = *pA;<br />
	pAnim[a].IsUsed();<br />
	return a;<br />
}<br />
<br />
<br />
<br />
<br />
void ParticleEngine::RemoveAnimation(int index) {<br />
	pAnim[index].DestroyParticleAnimation();<br />
}<br />
<br />
[/code]<br />
<br />
<br />
<br />
I've commented references to equations in the attached word document for those who can't understand the calculations.<br />
<br />
Just have a look, see what you think. Tell me what should be changed.]]>
		</description>
		<author>no-reply@allegro.cc (The Master)</author>
		<pubDate>Sat, 23 Sep 2006 16:29:36 +0000</pubDate>
	</item>
	<item>
		<description><![CDATA[<div class="mockup v2"><p>Alright, here are the things that come to mind when skimming through your sources. Please note that this is not a complete (or even formally correct) code review, so they&#39;re just suggestions/ideas you can consider or throw away.</p><div class="source-code snippet"><div class="inner"><pre><span class="c">// particle animation types</span>
<span class="p">#define EXPLOSION    AL_ID('A','E','X','P')  // the particles are forced outward</span>
<span class="p">#define IMPLOSION    AL_ID('A','I','M','P')  // the particles are drawn inward</span>

...

<span class="k1">int</span> anim_type<span class="k2">;</span>    <span class="c">// Either EXPLOSION or IMPLOSION</span>
<span class="k1">int</span> prim_type<span class="k2">;</span>    <span class="c">// either LINE, SPRITE, or CIRCLE</span>
</pre></div></div><p>

This looks like a perfect application for an <tt>enum</tt>. Generally, try to avoid preprocessor <tt>#define</tt>s where you can.</p><p>Also, use consistent nomenclature. I&#39;ve seen <tt>anim_type</tt> and <tt>maxDistance</tt>. Unless you have a very good reason to distinct those two by using different styles, I suggest you choose one and stick to that.</p><div class="source-code snippet"><div class="inner"><pre><span class="k1">int</span> CreateParticleAnimation<span class="k2">(</span><span class="k1">int</span> maxp, <span class="k1">int</span> col1, <span class="k1">int</span> col2, <a href="http://www.allegro.cc/manual/BITMAP" target="_blank"><span class="a">BITMAP</span></a> <span class="k3">*</span>t, Sprite <span class="k3">*</span>s, <span class="k1">float</span> f, <span class="k1">int</span> d, <span class="k1">int</span> at, <span class="k1">int</span> pt, <span class="k1">int</span> x, <span class="k1">int</span> y, <span class="k1">float</span> mSize, <span class="k1">float</span> mDis<span class="k2">)</span><span class="k2">;</span>
</pre></div></div><p>
This has too many parameters for me. Rule of thumb is to limit parameters to 7, the less, the better. There might be need for more complex structures/patterns to circumvent this, so it&#39;s probably easier to just keep it as-is.</p><p>This one&#39;s mostly stylistic: I always prefer the order <tt>public</tt>, <tt>protected</tt>, <tt>private</tt> to foster the idea of an interface. Unfortunately, C++ doesn&#39;t allow truely hidden members. There&#39;s the Cheshire cat pattern (also known as PImpl), but that&#39;s still not &quot;it&quot;. Think about what might be of interest to someone just using the class (not extending it). Those things are best put at the top, there&#39;s no need to look at <tt>private</tt> members.</p><p>Hmmm ... why <tt>CreateParticleAnimation</tt> and <tt>DestroyParticleAnimation</tt>? Sounds like a constructor/destructor pair. Maybe you don&#39;t need those methods at all.</p><div class="source-code snippet"><div class="inner"><pre>  <span class="k1">bool</span> InUse<span class="k2">(</span><span class="k2">)</span> <span class="k2">{</span> <span class="k1">return</span> used<span class="k2">;</span> <span class="k2">}</span>
  <span class="k1">void</span> IsUsed<span class="k2">(</span><span class="k2">)</span> <span class="k2">{</span> used <span class="k3">=</span> <span class="k1">true</span><span class="k2">;</span> <span class="k2">}</span>
  <span class="k1">void</span> UnUsed<span class="k2">(</span><span class="k2">)</span> <span class="k2">{</span> used <span class="k3">=</span> <span class="k1">false</span><span class="k2">;</span> <span class="k2">}</span>
</pre></div></div><p>
By reading the names of these methods, I would never guess the actual outcome. Try to use better names, especially <tt>InUse()</tt> and <tt>IsUsed()</tt> are confusing. On a similar note, you can make <tt>InUse()</tt> a <tt>const</tt> method, like so: <tt>bool InUse() const { ... }</tt>. Rule of thumb is to make as much <tt>const</tt> as possible.</p><div class="source-code snippet"><div class="inner"><pre><span class="k1">if</span><span class="k2">(</span>texture<span class="k2">)</span>
    <a href="http://www.allegro.cc/manual/destroy_bitmap" target="_blank"><span class="a">destroy_bitmap</span></a><span class="k2">(</span>texture<span class="k2">)</span><span class="k2">;</span>
</pre></div></div><p>
Two things: One, after you <tt>destroy_bitmap</tt> the texture, set it to 0. Two, and this is a very pedantic style issue, compare <tt>texture</tt> to 0. Everything not a boolean should be compared to a value. Someone reading <tt>if(texture)</tt> might deduce that <tt>texture</tt> in fact is a valid pointer inside its block, but that would be misleading (mind dangling pointers).</p><p>The whole type-dependent stuff (line, sprite, circle, and implosion/explosion) looks like a perfect application for polymorphism. You could extract the relevant behaviour to classes of their own, thus encapsulating functionality more. Something to think about <img src="http://www.allegro.cc/forums/smileys/wink.gif" alt=";)" />.</p><div class="source-code snippet"><div class="inner"><pre><span class="k1">void</span> ParticleAnimation::SetScreenPosition<span class="k2">(</span>point_t <span class="k3">*</span>p<span class="k2">)</span> <span class="k2">{</span>
  position.x <span class="k3">=</span> p-&gt;x<span class="k2">;</span>
  position.y <span class="k3">=</span> p-&gt;y<span class="k2">;</span>
<span class="k2">}</span>
</pre></div></div><p>
This one&#39;s unsafe. What happens if <tt>p</tt> is 0? Also, you make a deep copy anyway, so I suggest passing <tt>p</tt> as <tt>const point_t &amp;</tt>.<br /><b>(edit:</b> Even better would be to let <tt>point_t</tt> handle the copying, and just do <tt>position = p;</tt> in <tt>SetScreenPosition</tt>. If you don&#39;t have any pointers or sensitive data in <tt>point_t</tt>, this works out of the box. If you do, you&#39;re well advised to implement <tt>operator =</tt> yourself.<b>)</b></p><div class="source-code snippet"><div class="inner"><pre>pAnim <span class="k3">=</span> <span class="k2">(</span>ParticleAnimation<span class="k3">*</span><span class="k2">)</span><a href="http://www.delorie.com/djgpp/doc/libc/libc_551.html" target="_blank">malloc</a><span class="k2">(</span><span class="k1">sizeof</span><span class="k2">(</span>ParticleAnimation<span class="k2">)</span> <span class="k3">*</span> nP<span class="k2">)</span><span class="k2">;</span>
</pre></div></div><p>
Hm. Unless you want to the plain pointer approach for educational purposes, I suggest you switch to an STL container, like <tt>std::vector</tt> or <tt>std::list</tt>. There are already enough resources dealing with those (also here in the forum), so I won&#39;t go into detail here.</p><p>Well, that&#39;s about it what came to my mind. Further comments would require better analysis of your code&#39;s structure, which I cannot afford time-wise right now. Hope you can make some sense of my drivel.</p><p>Good luck with your particle engine!
</p></div>]]>
		</description>
		<author>no-reply@allegro.cc (Indeterminatus)</author>
		<pubDate>Sat, 23 Sep 2006 17:06:40 +0000</pubDate>
	</item>
	<item>
		<description><![CDATA[<div class="mockup v2"><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
This looks like a perfect application for an enum. Generally, try to avoid preprocessor #defines where you can.
</p></div></div><p>
Haven&#39;t used enums in ages, but I might look into that. I don&#39;t really remember how to use them, but i&#39;ve got plenty of books and info on the subject.</p><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
Hmmm ... why CreateParticleAnimation and DestroyParticleAnimation? Sounds like a constructor/destructor pair. Maybe you don&#39;t need those methods at all.
</p></div></div><p>

In CreateParticleAnimations, there is memory allocated, and if an error occurs the error message is returned. I generally don&#39;t like using malloc or any of those kinds of routines in constructors. But perhap I can put the DestroyParticleAnimation in a destructor.</p><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
Two things: One, after you destroy_bitmap the texture, set it to 0. Two, and this is a very pedantic style issue, compare texture to 0. Everything not a boolean should be compared to a value. Someone reading if(texture) might deduce that texture in fact is a valid pointer inside its block, but that would be misleading (mind dangling pointers).
</p></div></div><p>

Alright, so you mean if texture == NULL? Fair enough. Also, I&#39;ve found that if i tried to destroy a null bitmap it gave a massive runtime error. A lot of my programming habits have come from my experiences with DirectX, hence my use of classes and such.</p><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
By reading the names of these methods, I would never guess the actual outcome. Try to use better names, especially InUse() and IsUsed() are confusing. On a similar note, you can make InUse() a const method, like so: bool InUse() const { ... }. Rule of thumb is to make as much const as possible.
</p></div></div><p>

What purpose does const serve when it comes to functions? Seriously, I don&#39;t know. Furthermore, these are for internal use by the ParticleEngine, and not intended to be called by another class or function.</p><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
Hm. Unless you want to the plain pointer approach for educational purposes, I suggest you switch to an STL container, like std::vector or std::list. There are already enough resources dealing with those (also here in the forum), so I won&#39;t go into detail here.
</p></div></div><p>

That was a oversight on my part. usually i only use malloc for structures, and the new keyword for classes. but i&#39;ve not needed an array of classes for a while, so malloc has become second nature. Would a better approach be:</p><div class="source-code snippet"><div class="inner"><pre>pAnim <span class="k3">=</span> <span class="k1">new</span> ParticleAnimation<span class="k2">[</span>nP<span class="k2">]</span><span class="k2">;</span>
</pre></div></div><p>

By the way, did you see the thread i started about how much I hate RLE sprites? I used vectors fo those, and it gave me runtime errors all over the place. And i generally prefer not to go there as they&#39;re just too bloody hard to understand and there are no coherant tutorials i can find on the net. I prefer malloc or new.</p><p>Thanks for your comments. Just so you know, I am not intending on making this a user-friendly Particle Engine; it is purely another building block in my current programming project. And since I won&#39;t be releasing the final source code for the game, I don&#39;t see any reason to make the nomenclature neat and clean. Not to sound rude. However, these comments have been found useful in my own analysis.
</p></div>]]>
		</description>
		<author>no-reply@allegro.cc (The Master)</author>
		<pubDate>Sat, 23 Sep 2006 18:05:08 +0000</pubDate>
	</item>
	<item>
		<description><![CDATA[<div class="mockup v2"><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
What purpose does const serve when it comes to functions? Seriously, I don&#39;t know. Furthermore, these are for internal use by the ParticleEngine, and not intended to be called by another class or function.
</p></div></div><p>

It makes the <tt>this</tt> pointer <tt>const</tt>, meaning you&#39;ll get a compile error if you try to modify any non-<tt>mutable</tt> members. The purpose of this is that a method declared as <tt>const</tt> <i>guarantees</i> that it doesn&#39;t change the object&#39;s state. If those methods are intended for internal use only, you should hide them by making them <tt>private</tt>.</p><div class="quote_container"><div class="title">Quote:</div><div class="quote"><p>
And since I won&#39;t be releasing the final source code for the game, I don&#39;t see any reason to make the nomenclature neat and clean. Not to sound rude.
</p></div></div><p>

Not sounding rude, no worries <img src="http://www.allegro.cc/forums/smileys/smiley.gif" alt=":)" />. After all, it&#39;s <i>your</i> project, not mine, so do whatever you see fit. However, for your own sake, I want to stress the point of consistent nomenclature for future work. As a programmer, you shouldn&#39;t have to look up whether a specific variable&#39;s name was <tt>max_size</tt> or <tt>maxSize</tt> (for example). With consistent naming, you&#39;ll never run into such situations. Let the code rest for a few days, and it will become an issue if you have to extend/modify it later on.</p><p><b>(edit:</b> The bottom-line being that you shouldn&#39;t worry about &quot;low-level&quot; stuff like this. Better break your head over algorithms. <b>)</b>
</p></div>]]>
		</description>
		<author>no-reply@allegro.cc (Indeterminatus)</author>
		<pubDate>Sat, 23 Sep 2006 18:33:02 +0000</pubDate>
	</item>
</rss>
