Making one value for an int override another,,,

Newbie & Debugging Questions
#1

Hello guys I am not sure if this is the correct thing to be asking but here I go: So first off here is my code


	public static void block(SpriteBatch batch, float blockX) {
		batch.draw(Assets.stoneShort, movement + blockX, 80);
		// collision detection
/*these seemingly random numbers are just here for collision detection there the boundaries necessary to get this to work*/
		if (movement + blockX <= 165 && personY <= 160 && !(80 > movement + blockX + 103)) {
			if (movement + blockX <= 165 && !(movement + blockX <= 163) && personY <= 130)
				move = false;
			else
				move = true;
			if (move == true && personY > 134) {
				floor = 135;
				jump = false;
			}
		} else
			floor = 80;
	}

If I call this method only once everything works great collision detection is perfect and everything works. It gets messy when I call it more then once. So what I believe the problem is, is that the else{ floor = 80}; is overriding the if (move == true…){ floor = 135…} Since this is a sidescroller and if i call this method by


		Objects.block(batch, 1000);
		Objects.block(batch, 1500);

(Floor defines the Y value that the player stands on 0,0 starts at bottom left and goes up)
The first one has no collision detection on the top which is because of the problem i had just said about 80 overriding the 135, but on the second block (with the coordinates of 1500) the collision detection works great, so this further leads me to believe that ^^^ is the problem. So is this possible to make one number overide the other? or is there something wrong with my code :frowning:

#2

I really can’t tell what this does, and the description of the problem is that it “doesn’t work”. Your code is full of magic numbers that don’t carry any meanings. Where did you get 103, 163, 135 and all that? You need to assign these to constants or variables for the mnemonic value alone, not just so you can change them.

Help others help you debug.

#3

The meanings of those are in my post
And there actually was a description of what the problem is…

#4

What if you turn move into a local variable?

#5

You’re operating with too many global variables. “floor” doesn’t look like it’s supposed to be global, and if it is, you’re using it in a not so savvy manner.

If you need to use the information generated in this method, like “floor”, “move” and “movement” (which are other global variables that seem weird to have), you should make your method return these values, either by returning the value, or returning a data-object (an object that does nothing but hold the information you put in it).

It is generally a sign of rushing, if a method doesn’t return anything, while changing several global variables. It quickly gets very confusing to debug, which is what you’re seeing now.

Your method-name “block” also doesn’t say anything about what the method actually does, but it seems you’re doing drawing and collision-detection in the same method. This isn’t not the way to do it, but it is another thing adding to the confusion. Try to keep things separated.

A gameloop for a simple game normally goes like this:
1 React to input
2 Move entities
3 Do collision detection and corrections
4 Draw
5 goto 1

TL;DR Prefer the use of local variables in methods, and name variables and methods what they are/do. Makes debugging so much easier. You can always refactor, if you think of a better name later.

Hope this helps.

#6

I understand what you are saying ,so you think the task I am trying to accomplish is something I shouldnt do? rather i should follor your advice and do it from that? orrr…

#7

I think you should consider how you’re doing things now, and then how you can convert that to the 5-step process in my last post. Do a good bit of thinking on this, and back up your project, before changing it.

Generally we say “don’t try to fix what isn’t broken”, but it seems you’re running into problems with a broken framework, so in this case I’d recommend a good refactoring (rethink and rebuild of your setup). I once spent 16 hours rebuilding my framework, without having any idea if my game would work when I had finished…to my amazement, it worked perfectly (after 2-3 minor bugs had been addressed). Don’t be afraid. You have a back up :slight_smile:

You’ll want to end up with a gameloop consisting of very few method-calls. Something like my render-method in my LevelScreen (implements Screen in libgdx):


		Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT | GL10.GL_COLOR_BUFFER_BIT | GL10.GL_DEPTH_BUFFER_BIT | GL10.GL_STENCIL_BUFFER_BIT);
		
		// A bit special, since the physicsWorld has an update/step method of its own
		// This is part of step 1
		physicsWorld.step(timeStep, velocityIterations, positionIterations);

		// step 1, 2 and 3, as myWorld handles controls, movement and collisions
		myWorld.update(delta);

		// step 4 - The lines below update the camera and draws the sprites
		camera.update();
		stage.getSpriteBatch().setProjectionMatrix(camera.combined);
		stage.getSpriteBatch().begin();
		renderer.render();
		stage.getSpriteBatch().end();
		super.render(delta);

My global variables in my LevelScreen class, which this snippet is from. All are instantiated in the LevelScreen constructor:

  • ‘renderer’ is an instance of WorldRenderer, instantiated in the LevelScreen constructor, with my stage’s SpriteBatch sent as a parameter (this way I can draw my game and the menus using the same SpriteBatch)
  • ‘stage’ is the instance of stage in my current screen (LevelScreen)
  • ‘camera’ is a standard orthographic camera
  • ‘myWorld’ is an instance of my own LogicWorld-class, which has all the gamelogic, collision-detection methods and collections of current enemies, projectiles and such
  • ‘physicsWorld’ is an instance of World from Box2D, a physics-engine implemented in libgdx
#8

“these seemingly random numbers are just here for collision detection there the boundaries necessary to get this to work” is not an explanation of what they are. As sprongie says, instead of mixing these numbers in the code, create named constants for these numbers. It will make it easier to change values if needed and it will make it easier for others to read your code and understand what you are doing. Also what are the meanings of the variables movement and floor? Is movement how far the player moved in the last tick, and floor the y position of either the screen or the block? It isn’t entirely clear. Proper code commenting is a good habit to get into. Especially if you are going to be asking other people to look over your code. What does it mean for move to be true? Does it mean the player can move because there is no collision or does it mean the player has to be artificially moved by the system because it is overlapping with a block?

For now, every variable you introduce should have a (brief) comment explaining what it is and its purpose.

[quote]I understand what you are saying ,so you think the task I am trying to accomplish is something I shouldnt do? rather i should follor your advice and do it from that? orrr…
[/quote]
It’s not that you shouldn’t do it, but that you should probably do it differently. For one, you should separate out your rendering code from your collision detection code. This gives better cohesiveness to your methods. There is something called the Single Responsibility Principle that is usually applied to classes but also applies to methods. It basically means that a given method should only be responsible for one thing. That “one thing” can take several steps but you shouldn’t have unrelated processes going on in the same method. In your block method you are doing two unrelated things: collision detection and rendering. Methods that follow the SRP are generally smaller, easier to understand, and easier to debug. They are also easier to give meaningful names to.

Another small thing but it is generally easier to not use “!” if you are doing < or > type comparisons. For instance your:


 if (movement + blockX <= 165 && personY <= 160 && !(80 > movement + blockX + 103))

works better as


// you use this enough times that it is worth calculating once and holding it in a meaningful variable
// (although I don't know what it truly means.
float blockMovement = movement + blockX; 

// Converted the ! expression to its opposite mathematical expression.
// Also moved the 103 to the other side of the comparison.
if (blockMovement <= 165 && blockMovement  >= (80-103) && personY < 160 ) {

Your:


       if (movement + blockX <= 165 && !(movement + blockX <= 163) && personY <= 130)
            move = false;
         else
            move = true;

can also be simplified to:


move = ! (blockMovement > 163 && blockMovement <= 165 && personY < 130);
#9

(about my last post)
That isn’t the perfect solution or anything. This is just to show you an implemented (and working) way of structuring a game, so you can quickly find bugs.

As an example, I know that if I have some problem with an entity moving too fast, I have to look at LogicWorld and/or the Entity’s class to find the problem. If I have a problem with something being rendered wrongly, I can start looking in the WorldRenderer. If things are colliding weirdly, I can have a look in LogicWorld, where there’s a method called handleCollisions(). If my player can only move left, up and down, I probably have a problem in LogicWorld, in my actuateControls() method. I could go on…

Do you get the picture? Go a little OCD on it :slight_smile:

#10

Yeah I unseated what you guys are saying. After I fix everything I will have to figure out how to fix this problem

#11

Well, sort of. After doing a revamp on your setup, I don’t think this will pose a problem anymore. So long as you remember: changing global variables in methods leads to confusion. Instead, you should change global variables in as few places as possible, and change them based on results returned from your various methods. It doesn’t ALWAYS have to be like this, but it is a good “general rule” to follow.

#12

What I’m trying to say, is that ex. the position of the player should not be a global variable in your level-class, but be a private variable in a Player-class, which is (per update) only changed when the player is moved, and maybe a second time if he’s colliding with something. Try to use local variables while calculating and only change the actual variables at the very end.

I usually put myself “inside the method” and “forget” about being able to reach the global variables, so I create a basic method header, and then I ask myself “What will my method need to accomplish this?”
Then I try to get as much of that information through parameters as I can, and make local variables for the things I think I’ll need in my calculations, that aren’t already available to me. Maybe I’ll get the playerX and playerY position, and the size of his collision-box. Then I might find that I need to know the center of the player, so I make a local variable for that, etc…

Then when I’m finished, I want to set the position inside a Player-object. Oops, I have no reference to it; well, better put it in the list of needed parameters, then. And it goes on. Use the parameters as a shoppinglist of things you’d like to use in your method. When you’re done, you might even find that you’ve never used a specific parameter, which might help you remember if you’ve missed something, or maybe it simply proved unneeded, in which case you can remove it.

In the above case, a brainfart made me forget that I needed the Player-object to set the position of it at the end. So after I’ve added Player to the parameter-list, the playerX and playerY positions end up being redundant, since we can get those from the Player object. No worries, though. A quick find/replace will change “playerX” to “player.getPosX()” and “playerY” to “player.getPosY()” throughout the method, and we’re done cleaning up.

#13

[quote]So what I believe the problem is, is that the else{ floor = 80}; is overriding the if (move == true…){ floor = 135…}
[/quote]
I had to look carefully, because the way things are laid out is kind of unusual. But as far as I can see, there is no way the ‘else’ statement can override the ‘if’ statement as in your quote. If your code “if (move==true…)” executes, that thread’s program flow will never reach the “floor = 80” code. That’s just simple branching.

The only way things might override as stated in your quote is if there are concurrent calls happening. In that case, your static method, and global variables might be getting messed up by two threads running the method at overlapping times, if each call is making use of global or static variables as opposed to running independently with instance variables.

Hard to say too much as we don’t see where “floor” is getting declared or what other outside processes might be affecting this or some of the other variables in your method.

[quote]After I fix everything I will have to figure out how to fix this problem
[/quote]
You don’t have to “fix everything” to make this code more readable which might help make the issues easier to spot.

#14

=>

move = blockMovement <= 163 || blockMovement > 165 || personY >= 130;
#15

Just one more question; is the way I’m doing it the “wrong” way or is it just generally harder to debug with the way I did it?

#16

Ok wreed first of all “overriding” in java means overriding inherited methods not setting a value of a variable (not that bad just saying).
Secondly you need to know the scope of your variables, surely you must have seen things like

public Main(int a, int b){
this.a = a;
this.b = b;
}

Use global variables FIELDS OF A CLASS when you want the variable to be accessible in every method of that class. Use local variables when you only need to use that variable within the method in which it is declared.