Top-down movement

I've been working on a Final Fantasy-ish game and am leaning toward more like 1 - 3. This is my attempt at map movement. It does work, but if anyone has any suggestions or critiques, I would like to hear them. For instance, is there a different way you would do something or is there a way I can clean up the code?

```
package com.stardust.main;

import java.awt.Graphics;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.awt.image.BufferedImage;

public class Player implements KeyListener
{
//Location
private int x, y;
//Size Variables
private int sx, sy;
//Movement tests
public boolean up = false, dn = false, lt = false, rt = false;
public int SPEED = 3;
//More movement test mToPMove() stands for map to player movement up, down, left, right
private boolean mToPMoveU = false, mToPMoveD = false, mToPMoveL = false, mToPMoveR = false;

public BufferedImage image;

public Player(int x, int y, int sx, int sy, BufferedImage image)
{
this.x = x;
this.y = y;
this.sx = sx;
this.sy = sy;
this.image = image;
}

public void update()
{
//here map moves because player is in center of map
if(up && !mToPMoveU)
{
Game.getnewBackground().y += SPEED;
}
if(dn && !mToPMoveD)
{
Game.getnewBackground().y -= SPEED;
}
if(lt && !mToPMoveL)
{
Game.getnewBackground().x += SPEED;
}
if(rt & !mToPMoveR)
{
Game.getnewBackground().x -= SPEED;
}
//check when map hits edges of frame, then start player movement
checkMapPosition();
if(up && mToPMoveU)
{
Game.getPlayer().y -= SPEED;
}
if(dn && mToPMoveD)
{
Game.getPlayer().y += SPEED;
}
if(lt && mToPMoveL)
{
Game.getPlayer().x -= SPEED;

The first thing I noticed is that you're lacking separation of concerns. Why does you Player class implement KeyListener? It should just represent the state of your player in the game without dealing with user input. That should be the concern of another class.

Why do you have public int SPEED = 3;? You should really pass this a parameter to the constructor (or to a modifier method). Public fields are almost always a bad idea. You usually don't want them so you can have a better control of who/how can modify them.

mToPMoveU, mToPMoveD, mToPMoveL, and mToPMoveR have a very poor naming. What about introducing an enum MoveDirection with Up, Down, Left, Right, (and maybe None)? That should help you make your code cleaner.

Why is image a public member?

What is the meaning of sx and sy? Their naming is cryptic and does not help me understand what is their purpose.

All the invocation to static methods in Game like Game.getnewBackground().y are a huge smell. Why do you need them? You should avoid almost at all static methods.
Probably what you need is an public Movement update() method, that returns information about the movement performed by the player. It should be called by Game, which should also compute the new position regarding the current background. It does not make much sense to have it in Player. Ditto for checkMapPosition, checkPlayerPosition, and reverseMove invocations.

You want a different class to handle input and rendering. That class should contain the render, keyTyped, keyPressed, keyReleased methdods. They clearly don't belong to Player.

I hope that these comments will give you some ideas on how to structure better your code.

StackExchange Code Review Q#66419, answer score: 8