# Why isn't my player movement code working properly?

I'm making a text-based RPG. One thing I've been trying to implement for a long time now is the ability for the player to move North, South, East, or West. I have my code placed in a MoveBehavior class, which exists in my main Character class.

The problem is that, while my algorithm should work in theory, the locations the player moves to don't change unless the MoveDirection is North/South. East/West, for whatever reason, remains to be the same.

I'll post the class I have (with comments), and then explain in detail further down:

public class MoveBehavior
{
const int MAX_EAST_WEST_INDEX = 1;
const int MAX_NORTH_SOUTH_INDEX = 5;
const int MIN_INDEX = 0;

//Property Getters...
public Location CurrentLocation { get { return this.currentLocation; } }
public int AreaIndex { get { return this.areaIndex; } }
public int LocationIndex { get { return this.locationIndex; } }
public readonly Character ClassOwner;
public readonly int CharacterID; //ID of Character this instance belongs to.
//Location Handlers
private Location currentLocation;
private int areaIndex, locationIndex;

//Jagged Array of Locations
private Location[][] locations;

public MoveBehavior() {} //Default ctor.

//Constructor takes ID of character, along with a reference to the character object holding it.
public MoveBehavior (int id, Character character)
{
this.CharacterID = id;
this.ClassOwner = character;

InitializeIndexes();
InitializeLocations();
}

//Public Methods...

public void Move(MoveDirection moveDir)
{
Location pastLocation = null; //Past location is null to ensure its initialization.

switch (moveDir)
{
case MoveDirection.North:
this.areaIndex++;
pastLocation = locations[areaIndex - 1][locationIndex];
break;
case MoveDirection.South:
this.areaIndex--;
pastLocation = locations[areaIndex + 1][locationIndex];
break;
case MoveDirection.East:
this.locationIndex++;
pastLocation = locations[areaIndex][locationIndex - 1];
break;
case MoveDirection.West:
this.locationIndex--;
pastLocation = locations[areaIndex][locationIndex + 1];
break;
}

if (locationIndex > MAX_EAST_WEST_INDEX || locationIndex < MIN_INDEX) //Check to ensure array indexes aren't out of bounds.
{
locationIndex = 0;
}
if (areaIndex > MAX_NORTH_SOUTH_INDEX || areaIndex < MIN_INDEX)
{
areaIndex = 0;
}

this.currentLocation = locations[areaIndex][locationIndex]; //Update location

TravelMessage(pastLocation, this.currentLocation, moveDir); //Output Message

}

void TravelMessage(Location pastLocation, Location currentLocation, MoveDirection moveDir)
{
int pastNorthSouthIndex;
switch(moveDir)
{

case MoveDirection.North:
pastNorthSouthIndex = areaIndex - 1;
break;
default:
pastNorthSouthIndex =  areaIndex;
break;
}

Console.WriteLine("{0} has moved {1} from Area {2}, {3} to Area {4}, {5}",
this.ClassOwner.Name,
moveDir.ToString(),
pastNorthSouthIndex,
pastLocation.Name,
areaIndex,
currentLocation.Name);
}

void InitializeIndexes()
{
this.areaIndex = 0;
this.locationIndex = 0;
}

void InitializeLocations()
{
int iterationCount = 0;

locations = new Location[6][];

for (int i = 0; i < 6; i++)
{
locations[i] = new Location[3];
for (int j = 0; j < 3; j++)
{
locations[i][j] = new Location("Location: " + iterationCount,
" ",
new List<Item>(),
new List<Npc>()
);
iterationCount++;
}
}

}
}


The idea is to use a jagged array. The first dimension of this array is what dictates where the player is on the North/South coordinate, while the second dimension takes care of the East/West movement.

I use two indexes, an areaIndex(North/South), and a locationIndex(East/West).

If I wish to move North, the areaIndex would increment by one; likewise, to move South, the areaIndex would decrement by one. To move East would require the locationIndex to increment by one; West would decrement the locationIndex by one.

The problem is that, while the code clearly states this in the Move(MoveDirection moveDir) method, nothing happens.

I use the following code:

public static class Program
{
static void Main(string[] args)
{

Character johnDoe = new Npc("ID01");

johnDoe.MoveCharacter(MoveDirection.North);
johnDoe.MoveCharacter(MoveDirection.North);
johnDoe.MoveCharacter(MoveDirection.West);
johnDoe.MoveCharacter(MoveDirection.East);
}
}


To provide this output:

John Doe has moved North from Area 0, Location: 0 to Area 1, Location: 3
John Doe has moved North from Area 1, Location: 3 to Area 2, Location: 6
John Doe has moved West from Area 2, Location: 6 to Area 2, Location: 6
John Doe has moved East from Area 2, Location: 6 to Area 2, Location: 7


The third line indicates the west movement. To further clarify the issue, John Doe should be moving from Area 2, Location: 6 to Area 2, Location 5. Yet, it doesn't. Thus, my main question is, why isn't this working properly? I have tried a number of different solutions, most of them revolving around using multidimensional and jagged arrays.

• Your location index is constrained to either 0 or 1. – user744 Aug 14 '11 at 19:34
• For those who haven't read the FAQ recently, it was changed to add this line: Would a professional game developer give me a better/different/more specific answer to this question than other programmers? If yes, the question belongs here - if no, the question belongs (maybe) on StackOverflow. – Cyclops Aug 15 '11 at 22:16
• @Cyclops, it wasn't recently changed, it was there since the beginning of the year. – Tetrad Aug 15 '11 at 22:41
• @Joe, the edit was inappropriate. If you wanted to pull the relevant chunks out and point out the programming issue, that belongs in an answer. – Tetrad Aug 15 '11 at 22:41
• Why was this edited to add a lot more code, when the problem was clearly apparent in the first and much shorter snippet? (Or is that what Tetrad is alluding to?) – Kylotan Aug 16 '11 at 0:04

Because of this:

if (locationIndex > MAX_EAST_WEST_INDEX || locationIndex < MIN_INDEX)
{
locationIndex = 0;
}


You are trying to move west from the west-most location, so locationIndex becomes -1 and is therefore reset to 0 by the following code:

        if (locationIndex > MAX_EAST_WEST_INDEX || locationIndex < MIN_INDEX) //Check to ensure array indexes aren't out of bounds.
{
locationIndex = 0;
}


There is few other things in your code which you might want to change:

MAX_EAST_WEST_INDEX is set to 1, which makes it impossible to visit some of the areas, I guess it should be set to 2.

The method you use to limit movement to be within bounds is a bit peculiar, as it is now if you go too far east or north you will "roll over" and be at the west-most or south-most location, but trying to go too far in the other directions you will simply be blocked. Unless this is actually the desired behaviour you should make 4 individual statements to handle each of the directions separately.