1
votes

Let me first try to explain what I'm trying to achieve.

I'm making a game where the player can pickup power-ups(which I called, for some reason, "Loot" in my game). So here's a really basic form of my Loot class:

public class Loot : Sprite
{
    //Properties and fields...

    public Loot(/*parameters...*/)
    {
    }

    public virtual OnPickUp(Player player)
    {
    }
}

Player and Sprite are classes from me. Here's my problem: my sprite class holds a position(a Vector2, a struct from XNA representing a vector) and from my understanding of how C# works, copying two sprites will copy the reference of eachother, therefor, whenever I change one's position, it will change the other one's too.

Whenever a loot should spawn, I have a class holding the possible loots that can spawn which returns a random one and the loot is copied and shown to the screen, but as soon as I get more than one of the same type of loot, it messes up because of the reference copy problem (both will share the same position and only one will be visible).

So then I tried creating a new Loot object whenever I needed to spawn a new loot, so I had to provide a constructor with multiple parameters to easily copy a loot this way: (Let's pretend toSpawn can be any type derived from the Loot class, like a HealthPack, or whatever)

Loot spawning = new Loot(toSpawn.Position, toSpawn.Color /*etc...*/);

It looked right until I decided to implement "OnPickUp". Since I create a new Loot object rather than the proper child class, the functionality of toSpawn's OnPickUp function would disappear and picking up a loot would end up doing nothing(it would only call the base class Loot's empty function).

Obviously, I am doing something wrong here. I don't have much programming experience, and even less with C#, so I really have no idea of how I can fix this. I tried using an Action< Player> object representing the OnPickUp functionality, it would work but still limit me quite a lot(since I have to pass a static function as an Action< Player>, therefor limiting me to use the information of the player parameter and preventing me from using the sub-class specific information).

So, finally, my question really is: What would be a better design to allow having sub-classes with overloaded functions, but still being able to clone the base class and its properties?

Thank you for your precious time.

If anything is not clear enough(and I know it's not), then just ask me in the comments and I'll try to describe my problem in more details.

4
It sounds like you just want to create a Clone or Copy virtual function in your base class and have the subclasses implement their own copying functionality. Am I missing something? - Gabe
Basically yes, but would I need to implement a "Copy" function for every sub-class of my Loot class? For HealthPack, WeaponSpeedIncrease, etc? - Jesse Emond
Your base class can just call MemberwiseClone and each subclass would only have to copy members that can't be copied (e.g. they must contain references to different Location objects). - Gabe
@Gabe You just saved me so much typing and code repetition... O_O Thanks, didn't know about that wonderful function. - Jesse Emond

4 Answers

4
votes

have your Loot class implement ICloneable

public abstract class Loot : ICloneable
{
    public virtual object Clone()
    {
        Type type = this.GetType();
        Loot newLoot = (Loot) Activator.CreateInstance(type);
        //do copying here
        return newLoot;
    }
}
3
votes

I think I must be missing something with this question because Vector2 is a structure and not a class. Any direct assignment of one Vector2 to another automatically creates a copy.

var v2a = new Vector2(10.0f, 20.0f);
var v2b = v2a; //`v2b` is a new instance of `Vector2` distinct from `v2a`.

Your issue sounds like the PossibleLoots class isn't randomizing the position of each Loot instance created.

I assume your game logic says something like "each level there shall be a maximum of 10 loots spawned", so your PossibleLoots class creates all the possible Loot objects at start-up but doesn't display one unless it is asked to "spawn a loot".

So, if that's right, isn't this the kind of thing you need?

public class PossibleLoots
{
    private IList<Loot> _loots = new List<Loot>();

    public PossibleLoots(int maxLoots)
    {
        for (var i = 0; i < maxLoots; i++)
        {
            _loots.Add(new Loot(this.GetRandomPosition()));
        }
    }

    private Vector2 GetRandomPosition()
    {
        // Your logic here to create suitable locations for loot to appear
    }

    // Rest of your `PossibleLoots` code
    // to spawn each `Loot` and to deplete the `_loots`
    // collection when the player picks up each `Loot`
}

Please let me know if I've missed the point of this question.

3
votes

Vector2 is a struct not a class, so your original problem sounds like it is due to holding two references to the same Loot object, rather than having two different Loot objects with references to the same position.

The class responsible for creating Loot objects should be creating new instances of the relevant Loot subclass, rather than creating the base class. In your design, Loot should probably be an abstract class or even an interface, and you should use the abstract factory pattern to create the desired type of Loot object.

For example:

class LootFactory
{
    const int numLootTypes = 3;

    public Loot CreateLoot(Vector2 position)
    {
        Random rand = new Random();
        int lootIndex = rand.Next(numLootTypes);
        if (lootIndex == 0)
        {
            return new HealthPack(position);
        }
        if (lootIndex == 1)
        {
            ...
        }
        ...
    }
}
0
votes

To me it really sounds like you're still going about this the wrong way. While the answer given would work, it's not something you should have to do.

If you want to spawn a new loot and you want it to be a random of the possible types of loot, your constructor of the Loot class should do all that work. There shouldn't be any need for cloning or ICloneable.

When a new loot object is created, it should randomly become the type of loot it's going to be, no need to keep a list of all possible types somewhere and clone from them. The new loot object would then have it's own position and behaviors and work exactly like you're expecting it to.

Or am I just misunderstanding what you're trying to accomplish?