[Destro224]

So, I've been writing myself a simple tile-based RPG/Top-Down shooter hybrid, and have discovered a problem in my approach. In an effort to make my code follow OOP and be as modular as possible, I made myself a simple Animation Manager that I could just drop into any entities that I may come up with later and would handle the task of determining what frame should be active and drawn by the entity when its' draw() method is called. I've stored the specific images for each frame within the Manager's structure to avoid a big and cluttered central repository of images and to avoid any dependencies that has created for me in the past.

However, to my knowledge this would load a duplicate set of image files every time I create a new object, which doesn't sound like a very good idea. This is the code for my simple Animation Manager and the container classes it uses to store the frames:

import java.util.*;
import java.awt.image.*;

public class AnimationManager {
    private ArrayList<Animation> animationList; //dynamic array of Animation(s)
    private int currentAnimation;
    private int currentFrame;
    private boolean isPaused;
    
    public AnimationManager() {
        currentAnimation = 0;
        currentFrame = 0;
        isPaused = false;
    }
    
    public void Update() {
        currentFrame++;
        if (currentFrame >= animationList.get(currentAnimation).GetNumberOfFrames()) {
            currentFrame = 0;
        }
    }
    
    public void AddNewAnimation(Animation new_animation) {
        animationList.add(new_animation);
    }
    public BufferedImage GetCurrentImage() {
        return animationList.get(currentAnimation).GetFrame(currentFrame).image;
    }
    public void ChangeAnimation(int new_currentAnimation) {
        currentAnimation = new_currentAnimation;
        currentFrame = 0;
    }
    public void Start() {
        isPaused = false;
    }
    public void Stop() {
        isPaused = true;
        currentFrame = 0;
    }
    public void Pause() {
        isPaused = true;
    }
}

public class Animation {
    private Frame frame[];
    
    public Animation(SpriteSheet new_sheet) {
        frame = new Frame[new_sheet.sprite.length]; //Create an array just big enough to hold all the sprites in the sprite sheet.
        
        for (int index = 0; index < frame.length; index++) { //For as many Frames as exist, load the corresponding image from the sprite sheet.
            frame[index].image = new_sheet.sprite[index];
        }
    }
    
    public Frame GetFrame(int frameNumber) {
        return frame[frameNumber];
    }
    public int GetNumberOfFrames() {
        return frame.length;
    }
    public void AddKeyFrame(int frameNumber, String new_keyFrame) {
        frame[frameNumber].isKeyFrame = true;
        frame[frameNumber].keyFrame = new_keyFrame;
    }
}

import java.awt.image.*;

public class Frame {
    public BufferedImage image;
    public boolean isKeyFrame;
    public String keyFrame; //Identifies the action to be performed when this frame is displayed.
    
    public Frame(BufferedImage new_image) { //Non-KeyFrame Constructor
        image = new_image;
        isKeyFrame = false;
        keyFrame = "";
    }
    public Frame(BufferedImage new_image, String new_keyFrame) { //KeyFrame Constructor
        image = new_image;
        isKeyFrame = true;
        keyFrame = new_keyFrame;
    }
}

How would I go about storing a single image that will be used by all instances of a given entity? How can I avoid a central repository that hands a reference out upon request?

I have a strong feeling that the static keyword should come into play here, but despite reading probably a dozen tutorials by now on how to use it, I'm still not sure how it might work. I mean, I get the concept of where you want to use it, but not quite the how.

Or perhaps I structured myself into a hole with the way I wrote it? This is my first attempt at animating sprites, and I'm building off of concepts I've learned from reading around, but I'm trying to avoid seeing actual code until I either get a system working myself or run up against a brick wall.

[ButchDean]

This is where knowledge of design patterns would have helped you out a great deal. Basically you will have a singleton object that manages the loading of your resources, then have other objects requiring those resources reference that singleton.

Google singletons to find out exactly what they are and how to implement them.

[Destro224]

Alright, so I've done some research on the singleton concept, such as referenced here. I'm now trying to determine at what level I should introduce a singleton. Should I be storing a static version of each Frame, or possibly each Animation, which is requested by the respective entity upon creation? However, that still seems to me like a roundabout way to describe what I'm doing here, so I must be missing something crucial.

I hope I'm not meant to make my Animation Manager a singleton, since that was exactly what I was trying to avoid with this code - a single object that manages a task for many entities from the outside. My goal is to provide all the entities with the ability to manage crucial tasks for themselves.

[ButchDean]

You're confusing what should be used using only inheritance with the Singleton design pattern. Regarding the animations and behaviors, those should be inherited from a base class that is abstract and has methods that must be defined in all derived classes. My point with the singleton is that it provides a single place to store assets that you have loaded to be referenced by multiple objects.

I hope that makes things clear.

[Destro224]

In the several days since I last posted, I've been pondering how to use a singleton to achieve the desired effect. I believe I've finally got the concept straight in my head, and coded myself a singleton to hold four player Animations, which can be accessed by any object of type Player by referencing a singleton instance containing them:

class PlayerResources {
    private static PlayerResources instance = null;
    private ArrayList<Animation> playerAnimations;
    
    private PlayerResources() {
        playerAnimations = new ArrayList<Animation>();
        loadPlayerAnimations();
    }
    
    public static PlayerResources getInstance() {
        if (instance == null) {
            instance = new PlayerResources();
        }
        return instance;
    }
    
    public Animation getPlayerAnimation(int animationIndex) {
        return playerAnimations.get(animationIndex);
    }
    private void loadPlayerAnimations() {
        BufferedImage tempImage = null;
        try {
            tempImage = ImageIO.read(new File("Sprites/soldier_blue.png"));
        } catch (IOException e) {
            System.exit(2);
        }
        
        //Takes tempImage, and loads each row into a SpriteSheet named tempSheet, which is then accepted by the constructor for
        //a new Animation named tempAnimation, which is in turn loaded into the ArrayList.
        SpriteSheet tempSheet;
        Animation tempAnimation;
        for (int animNumber = 0; animNumber < 4; animNumber++) {
            tempSheet = new SpriteSheet(tempImage.getSubimage(0, animNumber * 64, tempImage.getWidth(), 64), 9);
            tempAnimation = new Animation(tempSheet);
            playerAnimations.add(tempAnimation);
        }
    }
}

However, my implementation doesn't seem to be working. Whenever I try to load the instance into an empty PlayerResources object, I get these errors:

Quote

Exception in thread "AWT-EventQueue-0" java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
at java.util.ArrayList.RangeCheck(ArrayList.java:547)
at java.util.ArrayList.get(ArrayList.java:322)
at AnimationManager.GetCurrentImage(AnimationManager.java:30)
at Entity.Draw(Entity.java:37)
at GamePanel.paint(GamePanel.java:37)

(truncated, it keeps throwing errors all the way down the component hierarchy)

I'm not sure what I did wrong or didn't do at all. Am I misunderstanding how to use a singleton?

Since I'm not yet able to edit my posts, I'd like to add that the code inside loadPlayerAnimation() shouldn't be the problem (unless there's some weird issue happening with threads or resources not loading fast enough or something). I've tested that code and it should function flawlessly if and when it's called.

[stayscrisp]

Quote

Whenever I try to load the instance into an empty PlayerResources object

What do you mean? PlayerResources is a singleton, no?

[baavgai]

I'd step back from the singleton for a moment. See if your reading animation logic really works. Indeed, a separate class for that could help.

e.g.

class AnimationStore {
	private final List<Animation> list;
	private final int spriteCount;

	private AnimationStore(String filename, int spriteHeight, int spriteCount, int spritesPerSheet) throws IOException {
		this.list = new ArrayList<Animation>();
		this.spriteCount = spriteCount;
		BufferedImage tempImage = ImageIO.read(new File(filename));
		for (int i = 0; i < spriteCount; i++) {
			SpriteSheet tempSheet = new SpriteSheet(
				tempImage.getSubimage(0, i * spriteHeight, tempImage.getWidth(), spriteHeight),
				spritesPerSheet); // guessing on this one
			list.add(new Animation(tempSheet));
		}
	}
	
	public int getSpriteCount() { return spriteCount; }

	public Animation get(int i) {
		return (i<0 || i>=spriteCount) ? null : list.get(i);
	}
}

class PlayerResources {
    private static PlayerResources instance = null;
    private  AnimationStore playerAnimations;
    
    private PlayerResources() {
        try {
	        playerAnimations = new AnimationStore("Sprites/soldier_blue.png", 64, 4, 9);
        } catch (IOException e) {
        	// WTF?
            System.exit(2);
        }
    }
    
    public static PlayerResources getInstance() {
        if (instance == null) { instance = new PlayerResources(); }
        return instance;
    }
    
    public Animation getPlayerAnimation(int i) { return playerAnimations.get(i); }
}

Now you can test AnimationStore without worrying about issues with the singleton just yet. For extra credit, I'd read the actual height of the image and derive the spriteCount from that. This might offer other insights.

Hope this helps.

[Destro224]

baavgai, on 06 July 2012 - 04:23 AM, said:

I'd step back from the singleton for a moment. See if your reading animation logic really works. Indeed, a separate class for that could help.

I know without a doubt that it does, at least under any conditions I could think of. The block of code inside loadPlayerAnimations() was taken directly from code a block of code inside my Player() constructor which I've been working with for several days. I've had the full range of walking animations playing in response to movement for my character and everything. It's just that, when I tried to transform them into a singleton, something failed.

This is my known working code inside my Player() Constructor:

BufferedImage tempImage = null;
        try {
            tempImage = ImageIO.read(new File("Sprites/soldier_blue.png"));
        } catch (IOException e) {
            System.exit(2);
        }
        
        //Takes tempImage, and loads each row into a SpriteSheet named tempSheet, which is then accepted by the constructor for
        //a new Animation named tempAnimation, which is in turn loaded into animationManager.
        SpriteSheet tempSheet;
        Animation tempAnimation;
        for (int animNumber = 0; animNumber < 4; animNumber++) {
            tempSheet = new SpriteSheet(tempImage.getSubimage(0, animNumber * 64, tempImage.getWidth(), 64), 9);
            tempAnimation = new Animation(tempSheet);
            animationManager.AddNewAnimation(tempAnimation); //Only part that's different from the singleton version.
        }

baavgai, on 06 July 2012 - 04:23 AM, said:

For extra credit, I'd read the actual height of the image and derive the spriteCount from that.

The problem with doing that here, at least for me, is that not all my sprites are going to be the same height. The majority are, but a few, such as the player, are bigger than a regular tile. How could I use the height of the image to discern the number of sprites contained within? To my knowledge, I'd have to pass the height of each sprite, which would still have to be hard coded in some manner.

[Destro224]

stayscrisp, on 06 July 2012 - 01:42 AM, said:

Quote

Whenever I try to load the instance into an empty PlayerResources object

What do you mean? PlayerResources is a singleton, no?

From what I understand of singletons, an empty PlayerResources object serves as a reference to the actual singleton, once it's filled by the singleton's getInstance() method.

In other words: You can't create a "new" (new as in the java keyword) object of type singleton. You can, however, request a reference to the existing one. And that reference needs to be stored somewhere in order to be used by the requesting object, at least temporarily. Hence the empty PlayerResources object.

EDIT Oh, NOW I get the ability to edit posts. After I already created a double post.

This post has been edited by Destro224: 06 July 2012 - 12:18 PM

[ButchDean]

A singleton is not a data type, it is a design pattern for data encapsulation.

[Destro224]

ButchDean, on 06 July 2012 - 03:04 PM, said:

A singleton is not a data type, it is a design pattern for data encapsulation.

That much I understand. However, there seems to be a chronic shortage of singleton examples that show how data is actually supposed to be contained inside a singleton. Either that, or I'm not using the right search keywords in Google.

I appreciate the help, but could you perhaps tell me what part of the singleton concept I got wrong? Point me in a direction, possibly? I now know exactly what I want, (At least I hope so) which is a singleton that, in some way shape or form contains the four animations that currently make up my player, which I can reference. Right now all I can extract from your reply is that I perhaps created some sort of dependency that a singleton shouldn't have?

You don't have to tell me how to fix it, in fact I'd rather you didn't, I'd just like to know what I'm still doing incorrectly so that I have something to go off of.

This post has been edited by Destro224: 06 July 2012 - 05:21 PM

[stackoverflow]

Look into the singleton pattern and/or a static factory.

[ButchDean]

Destro224, on 07 July 2012 - 01:17 AM, said:

ButchDean, on 06 July 2012 - 03:04 PM, said:

A singleton is not a data type, it is a design pattern for data encapsulation.

That much I understand. However, there seems to be a chronic shortage of singleton examples that show how data is actually supposed to be contained inside a singleton. Either that, or I'm not using the right search keywords in Google.

I appreciate the help, but could you perhaps tell me what part of the singleton concept I got wrong? Point me in a direction, possibly? I now know exactly what I want, (At least I hope so) which is a singleton that, in some way shape or form contains the four animations that currently make up my player, which I can reference. Right now all I can extract from your reply is that I perhaps created some sort of dependency that a singleton shouldn't have?

You don't have to tell me how to fix it, in fact I'd rather you didn't, I'd just like to know what I'm still doing incorrectly so that I have something to go off of.

Have you Googled 'Singleton Design Pattern'? No point me explaining when the info is out there.

[Destro224]

ButchDean, on 06 July 2012 - 07:39 PM, said:

Have you Googled 'Singleton Design Pattern'? No point me explaining when the info is out there.

I have. Using several different search terms as well. I've read at least half a dozen of the results thoroughly by now and I continue to do so. I linked one of the results that I used in one of my earlier posts after you first told me to look for them. I had believed I'd come to an understanding of what they're meant to be used for. I can't say for certain that my understanding is correct. The only way I know how to test that understanding is to implement it.

I've been successful at implementing empty singletons, as all the tutorials I've Googled have shown me. However, I do not seem to be able to successfully have them contain any data.

My latest implementation has still failed, and despite several different rewrites and attacking the problem from any angle I could think of, I do not know why. Reading the singleton articles again has not given me any fresh ideas. That's why I came here, to hopefully have someone point out the error I was making, whether it be syntax or logic. I try to only post things after I've reasonably exhausted my options. I don't plan to stop looking, either, but if it's easy to find an example that contains meaningful data, could you post a link to it or even the particular search terms that yielded the result? I'm willing to do all the legwork as soon as I can find further ground to stand on.

This post has been edited by Destro224: 06 July 2012 - 08:27 PM

[ButchDean]

Can you post your latest implementation? We can't pinpoint what you don't understand without seeing code; no code, no help. We need that code that you've tried to implement.

(And for the record I mean a new attempt.)

This post has been edited by ButchDean: 07 July 2012 - 05:24 AM