do you think the definition of the changescreen function will lead to memory leaks?

I wouldn't worry about it. C# manages memory on its own with the garbage collector, and while memory leaks can happen I think that you'll be plenty safe. A bigger problem is that when there are no more references to an object it becomes eligible for garbage collection, which may not be what you want for something like a Room in a game (and especially one that might change as the player interacts with it).

Or is there a better way to do it?

This is something that's just about always true of any code. Programming is a pragmatic activity in the sense that if your code works as intended it's right enough. But in this case, I suggest the following as broad design approaches (take or leave these, as you like):

1. I don't like having each Room class (like Level1) as a direct descendant of DrawableGameComponent. Even though you will need to draw each Room, you don't need to use inheritance to make it happen. It can also force you, as it did in the code you posted above, to do some weird casting and makes for very unintuitive and hard to read/maintain code. You can manually call currentRoom.Draw the Game.Update method. If you really want to keep the inheritance from DrawableGameComponent, you can have an intermediate base class (Room, maybe) from which all individual rooms will inherit.

2. The ChangeScreen function seems overly engineered to me. In line with the above, I would prefer it to take a Room parameter rather than a DrawableGameComponent for clarity alone. I'm also not totally sure what you're planning to get by creating instances of Rooms on-demand this way or why you are using Type.GetType to identify the next room to be created. It's the kind of thing that's maybe a little hackish but gets the job done in a small demo-style project but will cause you headaches in the future, unless you have some specific reason for having set things up this way.

3. The function calls are well into spaghetti code territory. This too is something that works fine for a small test project but is a nightmare later. Game creates a Level1 object -> Game.Update itself calls Level1.Update (which is fine and normal) -> Level1.Update calls Game.ChangeScreen (this is less fine). Method calls shouldn't be reaching back and forth between distinct objects this way-- you're overly coupled and for no benefit. In general, you want calls to be as one-way as possible in terms of one class containing another as a field.

4. You may have a particular reason for on-demand generation of Rooms, but I'll urge you to consider an alternative approach. Because a room's contents, triggers, and other components might change as the game progresses creating previously visited rooms via their constructors can be a problem. The player removes an object from a room, for instance, so you don't want the object to be there again when the player backtracks through the same room. You can avoid this with fancy polymorphic constructors, but you might also have something like a container of Room objects which contains an instance of every room in the game and then access and update them as needed. That way you only have to deal with varying room states when saving and loading a game.

I think that the best general advice I can give is to try and think of your program as a set of discrete modules which plug into each other only and exactly as needed. It can be hard to see in a small test project like this one, because Game has so many things that are within its area of responsibility that it's easy to have Game do everything and only use other objects as data containers.

I am hoping to always have a reference to Room as I will use to it to define what should be drawn on the screen(main menus,game rooms) at any given time until the game quits.

I think I may have been unclear. When I say reference, I mean an appropriately typed variable which points to a memory address allocated when an object is instantiated. In the code you showed above, you have a field "room" which has Level1 assigned to it. As soon as your room variable switches to Level2 (in your ChangeScreen method), there are no more references pointing to the memory address where Level1 is. Not only does this mean that you can't access that object again (with no reference to that memory address, it's gone for all intents and purposes) but that object is also eligible for garbage collection (meaning that even if you knew the address, the data stored there could vanish at any time during execution).

There's no hoping on this one. Your design needs an explicit approach for accessing instances of variables whether or not they're drawn on-screen at any given moment. If you allow the active references to expire then your design will need an explicit approach for creating new instances correctly as needed.

I'm creating Rooms this way since, for the actual game, the Room.Update function will use event handlers to check if the user has clicked on a door sprite and change room in Game based on that. I have never used Activator.CreateInstance before and though it has a slew of overloaded constructors, the one I'm using requires a Type parameter. If I'm missing something please let me know.

It's not that you're missing anything so much as you're adopting a complex approach and I'm not sure that you are using (or are planning to use) that complexity for anything. If this is the case, then you should at least consider a simpler approach instead.

As for events, you define those in the class definition just like the fields, constructors, and methods. You then define methods and subscribe them to the event or events you want, and the methods will execute each time the event is raised. You don't need to fiddle with the Activator class at all, and if your plan is to use it only as a static factory class you may want to roll your own instead. There is no reason that you need Activator or any other class just to use events.

Where is Game.Update calling Level1.Update? I guess if I create Room as a separate class I should also call its Update function from Game.Update. Also, are you saying I should have changeScreen in the Room object? This will certainly change to the new room but would it be good practice to dispose the current Room from inside itself? I am passing a reference to the Game object with the variable game but would the following be ok if I am calling changeScreen from say inside Room.Level1?
- game.room = new Level2(game);
- this.Dispose();

My XNA is pretty rusty, but if I remember correctly when Game.Update runs it calls the Update methods of all game components. That's why you were seeing the effect of the Level1 method even though you weren't specifically calling any Level1 methods anywhere in the game loop. Conceptually you should want all active game objects (the ones that will be updating each cycle) to update at the same time, and since XNA already provides an Update method for the game loop that's a good place to do it.

I do not recommend putting the ChangeScreen method in Room. Your instinct was (and is) correct: doing so would be a bad and extremely messy idea. As your sample is laid out, Game is indeed the place where ChangeScreen should be. As your code base changes you should think of which class is "responsible" for managing screens. That may be Game, or it may be some other class.

But invoking Game.ChangeScreen from within a Room object is pretty dicey. My point was that any given Room object shouldn't be doing something like changing which screen is active in the Game object; as your code sample is laid out above this really should be handled by Game. A screen is changed, it does not do any changing of screens. That's a big break in encapsulation and produces tighter coupling than you need and for no benefit.

I didn't think about this so thanks. I guess I will have to create some kind of List/Dictionary in Game that keeps track of items/sprites that have been removed from a room and generate/draw/handle those items in the respective Room based on some status field in the list/dictionary. Possibly one like Inventory for items and another for removed sprites.

There are lots of ways you can do it, and it's a major design decision. It might help to rough out on paper a couple of sample rooms which have the kinds of features that you want the player to be able to interact with (like a take-able item, a flip-able switch, a clue that only appears after some plot event has been triggered) and think about different ways you might represent them as fields of that Room.

I'll suggest using an enum to indicate what kind of object an object is, and to indicate its state (present/taken, on/off, active/inactive). If you're using a list or dictionary you can have a generic Draw method that only renders the objects marked as present (or whatever) and not have to fuss with multiple containers with dynamic contents. But think about it before settling on an approach. Whatever design you choose will still need to have the implementation details figured out and will almost certainly need to be refactored as your project progresses.

I think I should create a ScreenManager for the game since it is essentially a change to different screens based on user input. Having read only the basics of XNA game programming online through piece-by-piece tutorials I thought I should read Microsoft® XNA® Game Studio 4.0: Learn Programming Now! by Rob Miles. Thought it might be a good stepping stone. Would you recommend it? It seems pretty basic(what with trying to teach C# for the uninitiated also) but may be a good way to get a handle on XNA game architecture.

If there are other books you think might be better, please do let me know. It's a pity that most of the info out there either online or in books seems to focus on 3d games or side scrollers. Point-n-click games seem to have definitely gone out of style and they were such fun too.

I don't really like Manager classes for most purposes, and for something like screens and screen changing you don't need one. But there's no reason you can't or shouldn't use one if you want, especially if that makes your program design clearer or more intuitive for you.

I haven't read the book so I can't give you an opinion on how good it is or isn't. If you want to use XNA in the future then it isn't a bad idea to get some material that will help you learn how the guts of it work. For now though, I think that it might be more of a distraction than a help to you. You may end up spending a lot of time reading and memorizing and not so much time programming. Actually coding is by far the best way to improve, far more than reading books, and obviously if you aren't coding you won't make much progress on your game.

The same goes for other books, particularly for the kind of project you're working on now. A key point to remember is that a point-and-click adventure game isn't some totally different animal than a 2D-side scroller or 3D-whatever. Point-and-click is a very simple case of applying the same general ideas about putting pictures on the screen and responding to input, and it's not that the others have information that's totally different so much as that they're focused on techniques you don't need for this.

Like I mentioned before, nearly all of the complexity in your current project will come from getting your hands dirty with program design and planning and producing content. By putting a picture on the screen and having it respond when a part of it is clicked you've already got the capability to do what you want.

I also realize it's not good practice to have all these members as public but that was just a temporary fix until I created accessors for them.

Giving fields accessors by default isn't much better than leaving them public. Don't worry about it for now.

I was trying to implement a ScreenManager class so I tried to load a texture from another class but I get a ContentLoad exeception: GraphicsDevice component not found. I am trying to load the texture in the LoadContent of the class. It wont work there but it does work if I put the Content.Load in the Update method of the class. However, thats probably not a good place to load the texture

I'd need more information to say for sure, but from my hazy memory of XNA I'll suggest you're calling MainMenu.LoadContent (which is in turn calling Content.Load) before you're creating the instance of GraphicsDeviceManager. That would also explain why it works when you put the Content.Load call in MainMenu.Update, because that would almost certainly be happening after the GraphicsDeviceManager instance is created. But I don't see where these things are happening in the code you posted, so I'm not 100% confident about it.

I notice that you have a lot of LoadContent methods sprinkled around in your classes. There's nothing wrong with that, but it seems wrong to me to be calling Content.Load from them. Content.Load loads an image into memory and is kind of resource-heavy. It's generally done once, when the game starts up, and loads something like a whole sprite sheet or sheets at that time. Other classes that need specific images from the sprite sheet assign some shape from the sheet to a variable (like a Texture2D), and that's the kind of thing I would expect to see in a class' LoadContent method. This is another "if it works, it's right" kind of thing, but it's something to keep in mind going forward.

Hi satsujin,

Sorry I didn't get back to this post earlier. I got slammed with real life stuff and have had about ten minutes to devote to something other than work/school over the last week.

For animation I think that you're conflating a couple of things. The standard way, as far as I'm aware, of animating stuff off of a sprite sheet is to have a sequence of pictures for a given animation. A game object should have a variable field like "currentFrame" to indicate what should be drawn onscreen, and when the object's Draw() method is called it renders currentFrame to the screen. For a sprite sheet, the current sprite image is a portion of the sprite sheet, in SFML generally a Rectangle which "cuts out" the picture to be drawn.

When an animation is triggered (like, the jump button is pushed) the currentFrame is updated to "cut out" the next image in the sequence over and over again until the animation is over. All of the rendering to the screen is done by the Draw() method. The only thing that is changing is what portion of the sprite sheet is assigned to the currentFrame variable. The key point to understand is that an animation is just a defined set of frames, usually with a particular frame (called a key frame) which is "neutral" between animations, for smooth transitions between different animations. Think of Mega Man standing still between running, jumping, dashing, etc., and you'll have the idea.

Positioning the sprite onscreen is totally separate from the animation. The sprite (say Mega Man, for example) should always have its position on the screen stored as a field. When the player pushes "right" on the input device, if moving to the right is valid then the "RunRight" (or whatever you want to call it) animation sequence starts and continuously updates the current frame to be drawn on the screen. Separately, but simultaneously, the position of Mega Man moves to the right.

Anything which changes the state of an entity in the game should probably be handled in the Update() method, and Draw() should be a pretty straightforward, un-fancy method which does just what it says.

The forums are the best way to reach me, given that a 24 hour turnaround time is the fastest I can realistically do. If you don't want to post back and forth feel free to send me a private message, although you'll get a better set of perspectives if you keep posting instead.