• Create Account

# Refactoring if-conditions for player turn code

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

8 replies to this topic

### #1Juliean  GDNet+   -  Reputation: 6024

Like
0Likes
Like

Posted 10 June 2014 - 01:08 PM

Hello,

I've got some code for a 2d game that orients an NPCs graphic towards the direction he is walking to. There are 4 directions (down, left, right, up), that have the values (0, 1, 2, 3) respectively (so 3 - direction will yield the other direction on that axis). It is possible for the NPC to walk straight, diagonally, and unevenly-diagonally (2 pixel on the x-axis, 1 on the y-axis, hope you know what I'm talking about). Now the code for mapping this movement to the direction is rather clumsy, just a big mess of if-statements:

unsigned int DirectionFromMove(const math::Vector2& vMove)
{
if(vDir.x > 0)
{
if(vDir.y > 0)
{
if(vDir.x > vDir.y)
return 2;
else
return 0;
}
else if(vDir.y < 0)
{
if(vDir.x > abs(vDir.y))
return 2;
else
return 3;
}
else
return 2;
}
else if(vDir.x < 0)
{
if(vDir.y > 0)
{
if(abs(vDir.x) > vDir.y)
return 1;
else
return 0;
}
else if(vDir.y < 0)
{
if(vDir.x < vDir.y)
return 1;
else
return 3;
}
else
return 1;
}
else
{
if(vDir.y > 0)
return 3;
else if(vDir.y < 0)
return 0;
}
}


I thought about it for a while, but didn't find any better way myself. I'm really curious, since in "production"-code I always end up with stuff like this. Is there some clever solution here to get rid of all the ifs?

### #2Madhed  Crossbones+   -  Reputation: 4089

Like
7Likes
Like

Posted 10 June 2014 - 01:31 PM

POPULAR

unsigned int DirectionFromMove(const math::Vector2& vMove)
{
if(vDir.x < vDir.y) {
// left or up
if (vDir.x > -vDir.y){
return 3;
}
else {
return 1;
}
}
else {
// right or down
if (vDir.x > -vDir.y){
return 2;
}
else {
return 0;
}
}
}


I hope this works... gone testing...

EDIT: Seems to work. What this does is separating the space by the function f(x) = x and after that by the function f(x) = -x testing which side of the half-plane the point lies in.

Edited by Madhed, 10 June 2014 - 02:15 PM.

### #3Juliean  GDNet+   -  Reputation: 6024

Like
0Likes
Like

Posted 10 June 2014 - 01:49 PM

I hope this works... gone testing...

Yep, this does indeed work, thanks a lot, thats a whole lot easier. Except that I somehow got up and down mixed up, at least its easier to fix in your version So the general advice for such monstrous if-statements would be "look for an easier algorithmn", or is there some recommandation in case there is no easier algorithmn scheme?

### #4BryanDube  Members   -  Reputation: 136

Like
7Likes
Like

Posted 10 June 2014 - 01:55 PM

POPULAR

unsigned int DirectionFromMove(const math::Vector2& vMove)
{
if(vDir.x < vDir.y) {
// left or up
if (vDir.x > -vDir.y){
return 3;
}
else {
return 1;
}
}
else {
// right or down
if (vDir.x > -vDir.y){
return 2;
}
else {
return 0;
}
}
}


I hope this works... gone testing...

EDIT: Seems to work. What this does is separating the space by the function f(x) = y and after that by the function f(x) = -y testing which side of the half-plane the point lies in.

Assuming that works you can be even more fancy if you want and do

unsigned int DirectionFromMove(const math::Vector2& vMove)
{
return (vDir.x < vDir.y) + 2*(vDir.x > -vDir.y);
}


Also untested...

### #5Madhed  Crossbones+   -  Reputation: 4089

Like
3Likes
Like

Posted 10 June 2014 - 02:13 PM

Yep, this does indeed work, thanks a lot, thats a whole lot easier. Except that I somehow got up and down mixed up, at least its easier to fix in your version So the general advice for such monstrous if-statements would be "look for an easier algorithmn", or is there some recommandation in case there is no easier algorithmn scheme?

I started by drawing a crude diagram and looking for a mathematical solution. Sometimes it's easier to visualize that way.

### #6Madhed  Crossbones+   -  Reputation: 4089

Like
3Likes
Like

Posted 10 June 2014 - 03:10 PM

I want to add something to my last post.

If you have a function that returns 4 distinct values but there are some code paths that return the same value this is a very good indicator that the function can be refactored into a simpler solution.

When doing calculations in vector space I'd always recommended to scribble up a diagram except for the simplest of cases.

### #7Khatharr  Crossbones+   -  Reputation: 7440

Like
0Likes
Like

Posted 13 June 2014 - 07:55 AM

unsigned int DirectionFromMove(const math::Vector2& vMove)
{
return (vDir.x < vDir.y) + 2*(vDir.x > -vDir.y);
}


Also untested...

Assuming that works you can be even more fancy if you want and do

That's something like what I usually go with in cases like this. If you can split the distinctions into ranges within a binary number it becomes easier to see. In this case you're only dealing with a pair of binary states, so it's really the simplest case.

This sort of touches on the field of "discrete mathematics". You may want to poke around for a cheap book on the subject if this kind of 'puzzle' interests you. Usually when your intuition is telling you that something can be simplified there's a 'clever' solution in there somewhere.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

### #8Glass_Knife  Moderators   -  Reputation: 8600

Like
6Likes
Like

Posted 13 June 2014 - 08:31 AM

POPULAR

Assuming that works you can be even more fancy if you want and do
unsigned int DirectionFromMove(const math::Vector2& vMove)
{
return (vDir.x < vDir.y) + 2*(vDir.x > -vDir.y);
}
Also untested...

That is a very elegant solution.  I hate it.  I didn't really have a problem with the first one, except it looked ugly.  But unless this function is in a time critical path, I would prefer a verbose, ugly "looking" method that is easy for someone who has never seen the code to know what's going on.

But drop this in front of someone with no context and ask them what it does, and they won't be sure.  I had to look at it after coffee to figure it out.

Please don't go into nerd rage.  It is an awesome solution from a computer science point-of-view. I would be proud.  I just want to point out that sometimes un-optimized and un-refactored code may not be optimized for speed, but optimized for life, making it easier to understand that algorithm at the cost of a few cycles.

I think, therefore I am. I think? - "George Carlin"
My Website: Indie Game Programming

My Book: http://amzn.com/1305076532

### #9Khatharr  Crossbones+   -  Reputation: 7440

Like
2Likes
Like

Posted 13 June 2014 - 12:30 PM

~snip~

To be honest, when I see long nested if-trees I mentally replace the whole thing with a picture of spongebob jumping up and down. I get why it's not great to do it all on one line around JPs, but something like:

int result = 0;
if(vDir.x < vDir.y)  {result += 0x01;}
if(vDir.x > -vDir.y) {result += 0x02;}
return result;

Is much preferred for me. It's actually a lot more to do with visual efficiency rather than performance efficiency. I can look at those three lines and sort out what's going on in a few moments, but I'm simply not interested in browsing through that whole if-tree to see what details are where.

This is kind of a poor example of what I mean, since this really is the simplest use case for this and it could easily go either way, but when you get to something where you're crapping out ordinals based on 14 different conditions, it can be a lot easier to sort out a one-page bit-field rather than a five page if-tree.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

PARTNERS