This site requires JavaScript, please enable it in your browser!
Greenfoot back

Comments for Metroid NES

Return to Metroid NES

mjrb4mjrb4

2009/4/26

Good start - at the moment he can still walk through the floor though instead of jumping on it ;)
linish22linish22

2009/4/26

Yes, any tips on how to fix that?
NintoNinto

2009/4/26

1st thing: It's not a he but a she. 2nd thing: you could use some whiles to fix that.
mjrb4mjrb4

2009/4/26

Without the source, it's hard to suggest anything specific...
MuZiKMuZiK

2009/4/26

Did you make the character model yourself?
Haha, Mjrb4, you said HE can still walk through the floor :D
linish22linish22

2009/4/26

No, I didn't make Samus. I got her at http://www.spriters-resource.com/nes/metroid/sheet/1774
mjrb4mjrb4

2009/4/27

Alright, alright, I'm constantly getting told off for the same reason when playing super smash bros... ;)
A new version of this scenario was uploaded on Tue Apr 28 01:03:04 UTC 2009
linish22linish22

2009/4/28

Source is here!
mjrb4mjrb4

2009/4/28

Well, whenever you move left or right you need to place a condition on that movement - let's say if we're approaching the wall in either direction, we don't want to move. (By approaching, I mean right on top of!) So, every time there's something like: setLocation(getX() + 5, getY()); It becomes: if(!approachWall()) setLocation(getX() + 5, getY()); As for the approach wall method, try this: public boolean approachWall() { int widthOffset = (Greenfoot.isKeyDown("left") ? -1 : 1)*getImage().getWidth()/2; for(int i=-15 ; i<15 ; i++) { boolean result = getOneObjectAtOffset(widthOffset, i, Ground.class)!=null; if(result) return true; } return false; } First of all you have to determine which way you're going to look for the offset on the correct side - that's what the first line does. The funny statement in the middle (Greenfoot.isKeyDown("left") ? -1 : 1) is the equivalent of: int num = 0; if(Greenfoot.isKeyDown("left")) { num = -1; } else { num = 1; } ...and then using num in the widthOffset line instead of the tertiary operator. I should say though that while I'm all for it in situations like that where it can easily be understood, some people really aren't and think any use of it is bad coding style - so don't go too wild if you choose to use it. The first part is the condition, then there's a ?, then the value if the condition's true, followed by a :, and finally the value if the condition's false. Anyway, we've then got a loop in there to check along the height of the image - we want him to be stopped by walls anywhere along his height, not just at the top or the base. If we encounter a wall at any point, then return true - otherwise keep looking. If we don't find a wall anywhere along his height, return false (because we're not approaching a wall.) Few other bits I noticed in your code while I'm at it: - Your indentation isn't the worst I've seen, but it does seem a bit bizarre! Did you write it in the Greenfoot editor or somewhere else? Proper indentation really does make code easier to read and debug, if you write programs that start getting really large, then debugging when code is all over the place is a nightmare! - Along the same lines, javadoc comments for methods are also nice ;) - Thirdly, using == to compare strings won't work. You need to use .equals instead, so: shoot == "yes" would become shoot.equals("yes"). It's all to do with the two types of equality in Java - basically, == will check if they're the same string - i.e. exactly the same object in memory. .equals() will tell you whether the strings have the same contents. In 99.99% of cases, including here, it's .equals() that you want! (== does work occasionally because of Java doing clever performance things behind the scenes, but programatically speaking it's still not correct.) - In your metroidWorld class you're creating a lot of ground seperately - some for loops could condense that right down! - Finally and on the same lines as point 3, why are you using a string? It seems to me that the shoot field would be much better off as a boolean..! Unless you need more than two values for some reason. However, in that case I'd recommend a bit of reading on enums. Sure strings will work if you get it right, but if you misspell something or make a typo, the compiler won't catch it and you'll probably spend quite a while debugging before you notice what's gone wrong! Anyway, that was a bit of a mega comment (actually, what's the longest non-spam comment we've had on the gallery? Have I broken the record?) but I hope it's helped :-)
linish22linish22

2009/4/28

Yea... it helped. I'm working on it right now. And two more things. 1) The whole tabbing thing, I thought that looked nice! 2) The shoot == "yes", I was testing that but it didn't work any ways. I'm deleting the whole shoot thing, I'm going to do it later. I'm going to stick with how I made my Ground. You broke the record!
A new version of this scenario was uploaded on Tue Apr 28 14:58:25 UTC 2009
mjrb4mjrb4

2009/4/28

Ah don't get me wrong, I didn't mean get rid of the indentation completely! It was just absent in my examples above because the comments here truncate initial spaces. And to be honest, I'm really splitting hairs here because it's not necessarily essential and I've seen far far worse in terms of layout and code quality - but hey, you're welcome to take it or leave it! :-) I just meant that in the first Samus class you had, it was a bit inconsistent and (most of the time) you seemed to be working with blocks of 8 spaces rather than 4 (which is the generally accepted amount.) Generally speaking, the rule with indentation is to indent four spaces every time you open a code block, and take that indentation back 4 spaces each time you close one. So for example, in your metroidWorld class at the moment you've got: |public metroidWorld() |{ |            super(500, 450, 1); |                        addObject(new Samus(), 140,90); |                        addObject(new Ground(), 100,100); |                        addObject(new Ground(), 140,116); |                        addObject(new Ground(), 480,442); |                        //etc... |} ..which is fine up until the super() line, but then you seem to indent 8 more spaces for no apparant reason! Bringing the addObjects in line with the super() would be seen as more conventional and consistent. There are exceptions to the indentation rule, with long lines for example (more than 80 characters) some people (not me) like to break the line up into smaller chunks and indent it 8 spaces (to avoid confusion with the lines below indented at 4.) Some people (including me) also like to not indent temporary "debug" statements at all (like System.out.println()'s for checking the status of variables) because it makes it more easy to locate said statements for altering / removing. Also, by javadoc comments I mean ones similar in format to the one that appears at the start of the class: |/** | * Write a description of class Samus here. | * | * @author (your name) | * @version (a version number or a date) | */ You can also put these at the start of each method. For example, just before the approachWall() method you could put something like the following: |/** | * Determines whether Samus is approaching a wall on either side. | * The side that's checked is determined by which arrow key is being | * held down. | * | * @return true if Samus is approaching a wall, false otherwise. | */ Here we've got a description of the method first, and then the @return annotation which signifies what values the method returns and under what conditions. If you've got methods with parameters, you can describe these using @param followed by the name of the parameter (use it more than once if you've got more than one parameter.) There's lots of them, you can use things like @see if your method is related to another method, you've seen @author and @version at the top of the page - those are probably the most common ones you'll come across, but there are others. So what's the point of doing it that way? Quite simply, if you do that and then select documentation in the editor (instead of source code) you get nice little boxes and descriptions for each method based on what you've put in the comments. Javadoc comments always start with /** and end with */. (You can also start comments with /*, but they won't be javadoc ones.) At the end of the day though if you're just on here for a bit of fun programming and not aiming to take it too seriously, as long as your code's readable it shouldn't matter too much. If your project starts getting overly complicated, large, unreadable and unmaintainable then that's when you start to realise the benefits of proper javadoc comments and indentation! And likewise if you're aiming to take programming (in general) on to a higher level where you'll be working on things in teams, having a good, consistent coding style is very important for a number of reasons - and it pays to get into good habits early!
NintoNinto

2009/4/28

If you walk to the left and then keep tapping left then you'll go through the ground! :)
NintoNinto

2009/4/28

(Only works to the left...)
mjrb4mjrb4

2009/4/28

Oh and incidentally, you may notice that sometimes he falls into the ground and because of the new code, he can't move in either direction until he jumps first. This happens since he's technically in the ground, and so there's ground on both his right and left sides. The best way to fix this would probably be to prevent him falling into the ground in the first place - either looking ahead as he's moving (using getOneObjectAtOffset again to do this) or more simply to constantly check whether he's sunk into the ground, and if he has lift him out.
mjrb4mjrb4

2009/4/28

Hmm, you do indeed fall down on the left hand side! That's a bug in my code then... Are the images in the animation slightly different sizes? That might explain it.
linish22linish22

2009/4/28

Wow, two mega comments...
linish22linish22

2009/4/28

Lol you guys are my debuggers. Officially.
NintoNinto

2009/4/28

mjrb4, you said HE again! :]
A new version of this scenario was uploaded on Tue Apr 28 18:40:52 UTC 2009
linish22linish22

2009/4/28

Updated but I have a biiiig problem now... Walk off the edge and see that he goes halfway through. I don't know how to fix that. Let's just say I'm a big newbie.
mjrb4mjrb4

2009/4/28

Hmm. I think I need to work on my gender at sight skills... Well, an easy way to "solve" it would be: if(getOneIntersectingObject(Ground.class)!=null) { setLocation(getX(), getY()-1); } That'll also "cure" the problem of him falling through the ground. I use "" because when I say "solve" and "cure", instead of falling off or sinking into the ground then he'll rise up gradually instead. You could actually solve it (without inverted commas) by using the getOneObjectAtOffset() method to look ahead at the number of pixels that he'd move in the next act method, and then adjust it accordingly (i.e. if he's travelling at a rate of 10 pixels per iteration, but he's only 5 pixels away from the ground, change the speed to 5 pixels instead of 10. I'm going to be mean though and leave you to work out how to put that into code ;)
A new version of this scenario was uploaded on Wed Apr 29 01:38:11 UTC 2009
linish22linish22

2009/4/29

What does the getOneObjectAtOffset() do?
mjrb4mjrb4

2009/4/29

Have a look at http://www.greenfoot.org/doc/javadoc/, in the Actor class: "Return one object that is located at the specified cell (relative to this objects location). Objects found can be restricted to a specific class (and its subclasses) by supplying the 'cls' parameter. If more than one object of the specified class resides at that location, one of them will be chosen and returned." So it tries to find an object at an offset from the actor you're calling it on (x and y are used to specify the offset) and returns null if it can't find any.
linish22linish22

2009/4/29

So I say, getOneObjectAtOffset( x, y, Ground.class); return !=null; ?
mjrb4mjrb4

2009/4/29

Not exactly, because logically speaking the above doesn't really make sense. getOneObjectAtOffset returns an actor, and you need to store that somewhere to use it. What are you trying to do by returning != null? You can say: return getOneObjectAtOffset(x, y, Ground.class) != null; which is equivalent to: Actor a = getOneObjectAtOffset(x, y, Ground.class); if(a!=null) { return true; } else { return false; } ...or you could do: Actor a = getOneObjectAtOffset(x, y, Ground.class); return a != null; != means not equals, just like == means equals - you have to have two things to compare either side, otherwise it doesn't make sense and the compiler won't let it through.
linish22linish22

2009/4/29

Where should I put this?
mjrb4mjrb4

2009/4/29

Without meaning to put you off or sound condescending at all, it might help you to do some seperate reading up on various Java basics - primitive types, objects, types of methods, returning values, parameters etc. and what they do. I'm happy to keep throwing pointers at you but it's not really going to do much if you don't understand what the code does! In terms of the above code, you'd need to put that in a seperate method in a way that would function to determine whether you were about to sink into the ground. It's much like the approachWall() method, but this time the amount you "look ahead" and therefore feed to the getOneObjectAtOffset() method will depend on how fast you're travelling :-)
MuZiKMuZiK

2009/4/30

I could probably help you if your indentation wasn't so weird lol...look up standard loop/method structure and re-submit the scenario once you've fixed it.
MuZiKMuZiK

2009/4/30

Your approachWall() method doesn't work correctly. It always returns false no matter where samus is. Try writing this method with an if-else statement, and see what you get.
MuZiKMuZiK

2009/4/30

Hey, mjrb4. I tried this in his approachWall() method and got the same result. public boolean approachWall() { int widthOffset = (Greenfoot.isKeyDown("left") ? -1 :1)*getImage().getWidth()/2; for (int i=-15; i<15; i++) { boolean result = getOneObjectAtOffset(widthOffset, i, Ground.class) !=null; result = cake; if (poop = result) { return true; } else { return false; } } return cake; } Why is this?
MuZiKMuZiK

2009/4/30

Also, i noticed that you are adding many instances of ground. My suggestion would be to take it into GIMP or even paint and combine all the images into one.