Sign in to follow this  
ShoeStringGames

Wont read all my if statements correctly?

Recommended Posts

	if(newB1 > b1)
		{
			b1+=5*time;
			if(b1>newB1)
				b1=newB1;
		}
		else if(newB1 < b1)
		{
			b1-=5*time;
			if(b1<newB1)
				b1=newB1;
		}
		if(newB2 > b2)
		{
			b2+=5*time;
			if(b2>newB2)
				b2=newB2;
		}
		else if(newB2 < b2)
		{
			b2-=5*time;
			if(b2<newB2)
				b2=newB2;
		}
		if(newB3 > b3)
		{
			b3+=5*time;
			if(b3>newB3)
				b3=newB3;
		}
		else if(newB3 < b3)
		{
			b3-=5*time;
			if(b3<newB3)
				b3=newB3;
		}

For some reason, it seems like it wont even make them go up, only down :/

Share this post


Link to post
Share on other sites
Looks alright to me, too. Try using a break point in the debugger to verify the values are correct.

With all those ifs, it's a bit confusing what you're trying to accomplish with the code. Appears to be some kind of move-object-towards-target routine.

Just for inspiration, here's a different approach:
// Assumed semantics:
// b1 == current position
// newB1 == target position
float maxDistance = 5 * time;

// Calculate offset of target to current position
float offset = newB1 - b1;
// Limit to allowed movement speed
offset = min(max(offset, -maxDistance), maxDistance);
// Move
b1 += offset;

float offset = newB2 - b2;
offset = min(max(offset, -maxDistance), maxDistance);
b2 += offset;

float offset = newB3 - b3;
offset = min(max(offset, -maxDistance), maxDistance);
b3 += offset;


-Markus-

Share this post


Link to post
Share on other sites
Actually what I was trying to do was use three values to modulate color for a few images I have in game. I wrote a changeBoard(float time) function that would update the game slowly so it would fade into the color as opposed to popping to it.

The problem wasn't my IF statements that I rewrote about ten times, it was that I had created b1,b2,b3,newB1,newB2,newB3 as ints instead of floats. Not good!

Either way, I fixed it, thanks for your help!

Share this post


Link to post
Share on other sites
1) You have a bunch of corresponding pairs of variables, each of which you treat the same way. Why not instead keep them in arrays?


for (int i = 0; i < 3; ++i) {
if (newBs[i] > bs[i]) {
bs[i] += 5 * time;
if (bs[i] > newBs[i]) { bs[i] = newBs[i]; }
} else if (newBs[i] < bs[i]) {
bs[i] -= 5 * time;
if (bs[i] < newBs[i]) { bs[i] = newBs[i]; }
}
}


2) We can use references to clean that up a little bit and make it look like we're working with plain variables again:


for (int i = 0; i < 3; ++i) {
double& b = bs[i]; // or whatever data type it actually is
double& newB = newBs[i]; // or whatever data type it actually is
if (newB > b) {
b += 5 * time;
if (b > newB) { b = newB; }
} else if (newB < b) {
b -= 5 * time;
if (b < newB) { b = newB; }
}
}


3) The quantity '5 * time' is the same in either case, so we can determine it ahead of time:


for (int i = 0; i < 3; ++i) {
double& b = bs[i]; // or whatever data type it actually is
double& newB = newBs[i]; // or whatever data type it actually is
double delta = 5 * time;
if (newB > b) {
b += delta;
if (b > newB) { b = newB; }
} else if (newB < b) {
b -= delta;
if (b < newB) { b = newB; }
}
}


4) To "clamp" values, use the standard library algorithms std::min and std::max:


for (int i = 0; i < 3; ++i) {
double& b = bs[i]; // or whatever data type it actually is
double& newB = newBs[i]; // or whatever data type it actually is
double delta = 5 * time;
if (newB > b) {
b = std::max(b + delta, newB);
} else if (newB < b) {
b = std::min(b - delta, newB);
}
}


@VitaminCPP: to actually delete your multiple posts, check off the "Delete?" box when you edit the post.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
1) You have a bunch of corresponding pairs of variables, each of which you treat the same way. Why not instead keep them in arrays?

2) We can use references to clean that up a little bit and make it look like we're working with plain variables again:

3) The quantity '5 * time' is the same in either case, so we can determine it ahead of time:

4) To "clamp" values, use the standard library algorithms std::min and std::max:


1) Or even better, an STL vector.

2) Ingenious idea, I've never thought about using references that way. For me, that makes it instantly clear what you are trying to do.

3) I'd imagine things like that would be optimized in the compiler? Either case, it seems like a good habit.

4)What library is that in?

Share this post


Link to post
Share on other sites
Quote:
...would update the game slowly so it would fade into the color as opposed to popping to it.


So by the three 'B's do you mean red, green and blue?

If the above snippet of ur quote is what you want to do then you can do it a much different way (this may be not what u are trying to achieve mind you):

struct RGB { float r, g, b; }

RGB getFadedColor (RGB& start, RGB& end, float amount)
{
/* start = original color
end = target color
amount = a number from 0.0 -> 1.0 telling how much
to fade from start to end. */


RGB out;
/* we get the difference between start and end colors and
multiply the diff by the amount we want to fade then
add this value to the initial value - this is sign sensitive
and is a simple LERP (linear-interpolation) algorithm */


out.r = start.r + ((end.r - start.r) * amount);
out.g = start.g + ((end.g - start.g) * amount);
out.b = start.b + ((end.b - start.b) * amount);
}

// So if we use these initial colors
RGB start = { 0.25, 0.25, 0.75 };
RGB end = { 0.75, 0.75, 0.25 };

// 0.5 means interpolate halfway between the two colors
RGB fade = getFadedColor (start, end, 0.5);

// 'fade' should equal 0.5, 0.5, 0.5 since halfway between 0.25 and 0.75 is 0.5




Assuming thats what you wanted you can also trim down and refactor the code by doing this:
struct RGB { float r, g, b };

// this can act on any type of values
template <class T>
T lerp (T& a, T& b, T amount)
{
return a + ((b - a) * amount);
}

RGB getFaderColor (RGB& a, RGB& b, float amount)
{
RGB c = { lerp (a.r, b.r, amount),
lerp (a.g, b.g, amount),
lerp (a.b, b.b, amount) };
return c;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by ShoeStringGames

if(newB1 > b1)
{
b1+=5*time;
if(b1>newB1)
b1=newB1;
}
...
{
b3-=5*time;
if(b3<newB3)
b3=newB3;
}


For some reason, it seems like it wont even make them go up, only down :/



To get back to your original question, are you using signed or unsigned types?

Formatting your code correctly will also help tracking down these sort of errors. For example, even if your if statement is only one line, still enclose that line in { and }. If you come along later and modify your code so that you want to have two statements in your if, then it's very easy to forget to add the brackets at that stage.

Share this post


Link to post
Share on other sites
Quote:
Original post by Ultimape
3) I'd imagine things like that would be optimized in the compiler? Either case, it seems like a good habit.


It would almost certainly; but this isn't a question of optimization but of cleanliness. When we extract the value '5 * time', we document that the fact that we add 5 * time in one case and subtract it in the other is not simply coincidental; we say, i.e., that we intend that value to be the same in both cases, even if we later decide that the 5 should be, say, a 3 (or for that matter, a value calculated by some other process). It also means that when (not if ;) ) we do make that decision, we can't possibly forget to change it in both places, because there is only one place to change.

Quote:
4)What library is that in?


The standard C++ library. Oh, what header? <algorithm>.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
Original post by Ultimape
3) I'd imagine things like that would be optimized in the compiler? Either case, it seems like a good habit.


It would almost certainly; but this isn't a question of optimization but of cleanliness. When we extract the value '5 * time', we document that the fact that we add 5 * time in one case and subtract it in the other is not simply coincidental; we say, i.e., that we intend that value to be the same in both cases, even if we later decide that the 5 should be, say, a 3 (or for that matter, a value calculated by some other process). It also means that when (not if ;) ) we do make that decision, we can't possibly forget to change it in both places, because there is only one place to change.

Quote:
4)What library is that in?


The standard C++ library. Oh, what header? <algorithm>.


This is a far better way of approaching optimisation than merely assuming that the compiler will do everything for you.

This is what optimisation is all about when it comes to the programmer; giving the compiler the best possible chance of optimising your already optimised code.

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