Issue
EDIT
I am trying to create a 2D platform-game in Java. I have a Player-class that has an update()-method:
public class Player {
...
...
...
public void update(TileBlock[][] tb, ArrayList<MovingBlock> movingBlocks) {
// checkBlockCollision(TileBlock[][] tb)
// checkMovingBlockCollision(ArrayList<MovingBlock> movingBlocks)
// Bounds for collision
int x1 = (int) (x + width + GameState.xOffset);
int y1 = (int) (y + height + GameState.yOffset);
int x2 = (int) (x + GameState.xOffset);
int y2 = (int) (y + GameState.yOffset);
// Tile Block Collision
for (TileBlock[] tileBlocks : tb) {
for (int x = 0; x < tb[0].length; x++) {
if (tileBlocks[x].getID() == 1) {
// Right collision
if (Collision.playerBlocked(new Point(x1, y2 - 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1, y1 - 1), tileBlocks[x])) {
right = false;
}
// Left collision
if (Collision.playerBlocked(new Point(x2 - 1, y2 + 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x2 - 1, y1 - 1), tileBlocks[x])) {
left = false;
}
// Top Collision
if (Collision.playerBlocked(new Point(x2 + 1, y2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y2), tileBlocks[x])) {
jumping = false;
falling = true;
}
// Bottom collision
if (Collision.playerBlocked(new Point(x2 + 2, y1 + 1), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y1 + 1), tileBlocks[x])) {
y = tileBlocks[x].getY() - height - GameState.yOffset;
falling = false;
topCollision = true;
}
if (!topCollision && !jumping) {
falling = true;
}
} else if (tileBlocks[x].getID() == 2) {
// Right collision
if (Collision.playerBlocked(new Point(x1, y2 - 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1, y1 - 1), tileBlocks[x])) {
reachedGoal = true;
}
// Left collision
if (Collision.playerBlocked(new Point(x2 - 1, y2 + 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x2 - 1, y1 - 1), tileBlocks[x])) {
reachedGoal = true;
}
// Top Collision
if (Collision.playerBlocked(new Point(x2 + 1, y2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y2), tileBlocks[x])) {
reachedGoal = true;
}
// Bottom collision
if (Collision.playerBlocked(new Point(x2 + 2, y1 + 1), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y1 + 1), tileBlocks[x])) {
reachedGoal = true;
}
}
}
}
// Moving Block Collision
if (movingBlocks != null && !movingBlocks.isEmpty()){
for (MovingBlock movingBlock : movingBlocks) {
if (movingBlock.getID() != 0) {
// Right collision
if (Collision.playerMovingBlock(new Point(x1, y2 - 2), movingBlock) ||
Collision.playerMovingBlock(new Point(x1, y1 - 1), movingBlock)) {
right = false;
}
// Left collision
if (Collision.playerMovingBlock(new Point(x2 - 1, y2 + 2), movingBlock) ||
Collision.playerMovingBlock(new Point(x2 - 1, y1 - 1), movingBlock)) {
left = false;
}
// Top Collision
if (Collision.playerMovingBlock(new Point(x2 + 1, y2), movingBlock) ||
Collision.playerMovingBlock(new Point(x1 - 2, y2), movingBlock)) {
jumping = false;
falling = true;
}
// Bottom collision
if (Collision.playerMovingBlock(new Point(x2 + 2, y1 + 1), movingBlock) ||
Collision.playerMovingBlock(new Point(x1 - 2, y1 + 1), movingBlock)) {
y = movingBlock.getY() - height - GameState.yOffset;
falling = false;
topCollision = true;
// Move the player the same amount the block is moving
GameState.xOffset += movingBlock.getMove();
} else {
if (!topCollision && !jumping) {
falling = true;
}
}
}
}
}
topCollision = false;
double moveSpeed = 2.5;
if (right) {
GameState.xOffset += moveSpeed; // Move the tile-block to the left because the player is moving right
}
if (left){
GameState.xOffset -= moveSpeed; // Move the tile-block to the right because the player is moving left
}
if (jumping){
GameState.yOffset -= currentJumpSpeed;
currentJumpSpeed -= 0.1; // Jump is slowing down at 0.1 pixel per call to update()
if (currentJumpSpeed <= 0){
currentJumpSpeed = startSpeed;
jumping = false;
falling = true;
}
}
if (falling){
GameState.yOffset += currentFallSpeed;
// Fall-speed
double maxFallSpeed = 5;
if (currentFallSpeed < maxFallSpeed){
currentFallSpeed += 0.1;
}
}
if (!falling) { currentFallSpeed = 0.1; } // So we fall again without going too fast in the start
// Player/Enemy collision
if (Collision.PECollision() && !invincible) {
health--;
invincible = true;
startTime = System.currentTimeMillis();
}
if (invincible) {
int deathDuration = 2000; //in milliseconds
long duration = System.currentTimeMillis() - startTime;
if (duration > deathDuration){
invincible = false;
}
}
}
}
This code works, but update() is very big. So I am trying to extract the two collision-methods, so that I only call them in update(). But when I try to change the code above to this:
public void update(TileBlock[][] tb, ArrayList<MovingBlock> movingBlocks) {
checkBlockCollision(tb);
checkMovingBlockCollision(movingBlocks);
topCollision = false;
double moveSpeed = 2.5;
if (right) {
GameState.xOffset += moveSpeed; // Move the tile-block to the left because the player is moving right
}
if (left){
GameState.xOffset -= moveSpeed; // Move the tile-block to the right because the player is moving left
}
if (jumping){
GameState.yOffset -= currentJumpSpeed;
currentJumpSpeed -= 0.1; // Jump is slowing down at 0.1 pixel per call to update()
if (currentJumpSpeed <= 0){
currentJumpSpeed = startSpeed;
jumping = false;
falling = true;
}
}
if (falling){
GameState.yOffset += currentFallSpeed;
// Fall-speed
double maxFallSpeed = 5;
if (currentFallSpeed < maxFallSpeed){
currentFallSpeed += 0.1;
}
}
if (!falling) { currentFallSpeed = 0.1; } // So we fall again without going too fast in the start
// Player/Enemy collision
if (Collision.PECollision() && !invincible) {
health--;
invincible = true;
startTime = System.currentTimeMillis();
}
if (invincible) {
int deathDuration = 2000; //in milliseconds
long duration = System.currentTimeMillis() - startTime;
if (duration > deathDuration){
invincible = false;
}
}
}
private void checkMovingBlockCollision(ArrayList<MovingBlock> movingBlocks) {
// Moving Block Collision
if (movingBlocks != null && !movingBlocks.isEmpty()){
for (MovingBlock movingBlock : movingBlocks) {
if (movingBlock.getID() != 0) {
// Right collision
if (Collision.playerMovingBlock(new Point(x1, y2 - 2), movingBlock) ||
Collision.playerMovingBlock(new Point(x1, y1 - 1), movingBlock)) {
right = false;
}
// Left collision
if (Collision.playerMovingBlock(new Point(x2 - 1, y2 + 2), movingBlock) ||
Collision.playerMovingBlock(new Point(x2 - 1, y1 - 1), movingBlock)) {
left = false;
}
// Top Collision
if (Collision.playerMovingBlock(new Point(x2 + 1, y2), movingBlock) ||
Collision.playerMovingBlock(new Point(x1 - 2, y2), movingBlock)) {
jumping = false;
falling = true;
}
// Bottom collision
if (Collision.playerMovingBlock(new Point(x2 + 2, y1 + 1), movingBlock) ||
Collision.playerMovingBlock(new Point(x1 - 2, y1 + 1), movingBlock)) {
y = movingBlock.getY() - height - GameState.yOffset;
falling = false;
topCollision = true;
// Move the player the same amount the block is moving
GameState.xOffset += movingBlock.getMove();
} else {
if (!topCollision && !jumping) {
falling = true;
}
}
}
}
}
}
private void checkBlockCollision(TileBlock[][] tb) {
// Tile Block Collision
for (TileBlock[] tileBlocks : tb) {
for (int x = 0; x < tb[0].length; x++) {
if (tileBlocks[x].getID() == 1) {
// Right collision
if (Collision.playerBlocked(new Point(x1, y2 - 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1, y1 - 1), tileBlocks[x])) {
right = false;
}
// Left collision
if (Collision.playerBlocked(new Point(x2 - 1, y2 + 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x2 - 1, y1 - 1), tileBlocks[x])) {
left = false;
}
// Top Collision
if (Collision.playerBlocked(new Point(x2 + 1, y2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y2), tileBlocks[x])) {
jumping = false;
falling = true;
}
// Bottom collision
if (Collision.playerBlocked(new Point(x2 + 2, y1 + 1), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y1 + 1), tileBlocks[x])) {
y = tileBlocks[x].getY() - height - GameState.yOffset;
falling = false;
topCollision = true;
}
if (!topCollision && !jumping) {
falling = true;
}
} else if (tileBlocks[x].getID() == 2) {
// Right collision
if (Collision.playerBlocked(new Point(x1, y2 - 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1, y1 - 1), tileBlocks[x])) {
reachedGoal = true;
}
// Left collision
if (Collision.playerBlocked(new Point(x2 - 1, y2 + 2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x2 - 1, y1 - 1), tileBlocks[x])) {
reachedGoal = true;
}
// Top Collision
if (Collision.playerBlocked(new Point(x2 + 1, y2), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y2), tileBlocks[x])) {
reachedGoal = true;
}
// Bottom collision
if (Collision.playerBlocked(new Point(x2 + 2, y1 + 1), tileBlocks[x]) ||
Collision.playerBlocked(new Point(x1 - 2, y1 + 1), tileBlocks[x])) {
reachedGoal = true;
}
}
}
}
}
The collisions are not registered and the player just falls through the map and out of bounds instead of landing on the blocks on the map, as intended. What is the correct way to extract the method here? All help is appreciated!
Solution
I found a mistake that may be causing your problem.
TLDR; Solution
In the already refactored code, you seem to only init your bounding values, this part :
// Bounds for collision
int x1 = (int) (x + width + GameState.xOffset);
int y1 = (int) (y + height + GameState.yOffset);
int x2 = (int) (x + GameState.xOffset);
int y2 = (int) (y + GameState.yOffset);
where in the original code you seem to init them every time you go in the update() method.
So I believe you only have to move these inits where they belong to make it work.
Proper way to extract method in latest Intellij
Here is simple way to extract method from any code for Intellij:
- Select code you want to extract
- Right click, and then unfold
Refactor - Under
Refactor, you should seeExtract Method..., Left click it - then, you only have to follow the wizard to set method name, every params etc
EDIT
You added comment while I was responding so, couldn't know you already tried auto method extracting; But first part of my message still remain valid to fix bug
Answered By - jacouille
Answer Checked By - Mary Flores (JavaFixing Volunteer)
