0
\$\begingroup\$

In my XNA game, I'm have the enemy sprites loaded and in the game screen. However, I put in the code to update their position in the Update method of the main class. The objective is to get the sprites to move from the right hand side of the screen to the left, and the player has to jump over them. Here's my code in the update method(note, the enemy sprites are created in a Texture2d list, and their positions in a Vector2d list):

                Vector2 ep1 = enemyPosition[0];
                Vector2 ep2 = enemyPosition[1];
                Vector2 ep3 = enemyPosition[2];


                        if (enemyPosition[0].X < 800)
                        {
                            ep1.X--;


                        }

                        if (enemyPosition[1].X < 800)
                        {
                            ep2.X--;

                        }

                        if (enemyPosition[2].X < 800)
                        {
                            ep3.X--;

                        }
\$\endgroup\$
  • \$\begingroup\$ btw, I tried using a foreach loop, but it didn't work out. \$\endgroup\$ – John Leni May 18 '13 at 11:57
  • \$\begingroup\$ Have you tried following the official 2D tutorial? In addition to being really helpful if you are new to XNA or c# or programming, it also features enemy sprites moving from right to left across the screen. \$\endgroup\$ – Seth Battin May 18 '13 at 16:08
2
\$\begingroup\$

you change value of the variables - ep1, ep2, ep3, but you dont change objects coords.

For moving from right side to left side you can try anything like this (also you can use foreah):

for (int i=0; i<3; i++)
{
    if (enemyPosition[i].X > 0)
    {
        enemyPosition[i].X--;
    }
}

P.S. Sorry for my bad English.

\$\endgroup\$
1
\$\begingroup\$

Jesse Emond is correct. As for how to orient your code correctly; My best suggestion to solve your problem is to take a more class-oriented approach, even if just by a little bit. Rather than maintain a list of each enemy's position, maintain a list of enemies, each of which has a position. That way you could add other attributes to enemies too. Most importantly, modifying an Enemy's position directly is going to be much easier than trying to make sure you always maintain references to the same Vector2's. (if Enemy's are Objects, then you'll know a second one won't be made unless you call new). I'd even recommend giving Enemy's a "move(amount)" method.

\$\endgroup\$
1
\$\begingroup\$

In C#, there are two situations that might happen when you copy something, for example Vector2 position = positions[0];:

  1. If the object is a struct, it will copy the entire object. float is a structure, int is a structure, double is a structure. Basically, this quote from msdn sums it well: Structs are copied on assignment. When a struct is assigned to a new variable, all the data is copied, and any modification to the new copy does not change the data for the original copy.
  2. If the object is a class, it will be passed around as a reference. string is a class and you usually make classes. When passed to a function or assigned to an object, modifying the copied object actually modifies the original object itself, since it is a reference.

What does this mean to us? Well, we have to be very careful when we pass variables around. When we pass a structure, we must know that it is not actually modified (unless we use keywords such as ref or out). When we pass a class, we must know that the original object might be modified (although there are ways to clone a class).

Now, Vector2 is a structure. It implies that we can't iterate through it with a foreach loop (as you have tried) and that any copy will not modify the original object. So, when you do Vector2 ep1 = enemyPosition[0];, you make a copy and only modify the copied version, so the objects won't move.

Instead, modify the objects directly, and I would recommend using a for-loop instead of doing it line-per-line. This will allow you to modify it later and add/remove objects without needing to modify your code. Do it like so (assuming that enemyPosition is a List<Vector2>):

for (int i = 0; i < enemyPosition.Count; ++i)
{
    if (enemyPosition[i].X < 800)
    {
        enemyPosition[i].X--;
    }
}

By using the loop, you remove code duplication, which makes it easier to later modify the program and fix bugs without forgetting a piece of your code. Always try to remove code duplication when you can, it will make your code more flexible.

In the same order of ideas, if we want to make it easier to change the program later on you might want to use constants rather than magic values. The 800 here requires the reader of the code to implicitly understand what it represents, therefore making it less readable and less flexible if you use this value somewhere else. I would advise against magic values. Instead, you could do the following:

// Somewhere in your class...
const int ScreenWidth = 800;

// In your Update method...
for (int i = 0; i < enemyPosition.Count; ++i)
{
    if (enemyPosition[i].X < ScreenWidth)
    {
        enemyPosition[i].X--;
    }
}

After that, you could change to a design with classes instead, which will probably make it all much cleaner. See Katana314's answer for doing so.

\$\endgroup\$
  • \$\begingroup\$ So you're saying I have to use the keywords ref and out in this case? \$\endgroup\$ – John Leni May 18 '13 at 13:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.