Welcome, Guest. Please login or register.
Did you miss your activation email?

Login with username, password and session length

 
Advanced search

1075932 Posts in 44152 Topics- by 36119 Members - Latest Member: Royalhandstudios

December 29, 2014, 04:14:05 PM
TIGSource ForumsDeveloperTechnical (Moderators: Glaiel-Gamer, ThemsAllTook)Java code structure question
Pages: [1]
Print
Author Topic: Java code structure question  (Read 391 times)
Ralgarog
Level 0
***


View Profile
« on: May 29, 2013, 05:16:05 PM »

I am a relatively new programmer and i am currently in the process of making a game in Java using slick2d. I try my hardest to follow OOP standards and in doing so my program currently has over 23 classes, 15 of which have over 10 functions each.

I currently have over 4500 lines of code written for this basic game (Most of which is the collision detection i guess)

I currently do not have any problems finding anything and it is pretty easy for me to change individual parts but it seems like everything is growing out of control.

My question is: Is there a point where a program can be considered to have -too much- classes? Additionally: Is there a point where breaking (sometimes vaguely) dissimilar code into classes can become a hassle?

Is there someplace where i can find information on this if none of you are able to help?

I can provide my code on request.

Thank for reading.
Logged
Walt Destler
Level 0
**


Walt Destler


View Profile WWW
« Reply #1 on: May 29, 2013, 06:51:45 PM »

Your number of classes and functions doesn't seem remotely unreasonable to me, so I'm curious why "it seems like everything is growing out of control". Can you elaborate on why you feel that way? Knowing what you're having trouble controlling will help me help you.

Just for comparison, my current project has about 126,000 lines of code in 808 files, and even that pails in comparison to most AAA games.

If you would like to email me your code then I'm happy to take a look at it and provide constructive advice.
Logged
Ralgarog
Level 0
***


View Profile
« Reply #2 on: May 29, 2013, 07:25:47 PM »

I sent you a PM with a link to my code.

It is relatively hard for me to state exactly what i think is wrong. It just seems like it is growing rapidly even though it is a simple 2d game. I remember looked at some source code for a simple 2d game a few months ago and it was about 5 files and almost 3000 lines of code.

Maybe i am just being paranoid though. Thanks for the response. I greatly appreciate it.
Logged
CowBoyDan
Level 3
***


View Profile WWW
« Reply #3 on: May 30, 2013, 08:00:28 AM »

Do the names of your classes describe what they "all about?"  Do your function names describe what they do?  Do the functions operate on the data that is part of the class or do they tend to operate on the data passed in/out?  Part of OOP means you shouldn't have to keep all the pieces in your head.
Logged
Ralgarog
Level 0
***


View Profile
« Reply #4 on: May 30, 2013, 09:20:24 AM »

For the first and second statement, Yes they do with the exception of one. Some example names of my classes are:

Entity
Object
Tile
Physics_Handler

Classes like Physics_Handler (which i am going to refactor to PhysicsHandler) have other helper classes that does specific functions. There are also a few classes that are simply generic data types such as my CollisionData class as seen below:

Code:
package slick;
import org.newdawn.slick.geom.Vector2f;

public class CollisionData {
boolean collision, collisionX, collisionY;
boolean passable;
String collideType;
String pDataType;
Entity active;
float validX;
float validY;
Vector2f position;
void setCollisionX(boolean x){collisionX = x;}
void setCollisionY(boolean y){collisionY = y;}
final boolean getCollisionX(){return collisionX;}
final boolean getCollisionY(){return collisionY;}
public CollisionData(Vector2f position)
{
setCollision(false);
setCollisionX(false);
setCollisionY(false);
setPassable(true);
setCollideType("null_affector");
setValidX(position.x);
setValidY(position.y);
this.position = new Vector2f(position);
}

public boolean equals(String cType){ return (collideType.equals(cType));}
public CollisionData(boolean col, String cType)
{
setCollision(col);
setCollideType(cType);
}
void setValidX(float value){validX = value;}
void setValidY(float value){validY = value;}
void setActive(Entity e){this.active = e;}
void setPassable(boolean pass){this.passable = pass;}
void setCollision(boolean col){this.collision = col;}
void setCollideType(String cType){this.collideType = cType;}
final Entity getActiveItem(){return active;}
final boolean isPassable(){return passable;}
final boolean getCollision() {return collision;}
final String  getCollideType() {return collideType;}
final String getpDataType() {return pDataType;}
final float getValidX(){return validX;}
final float getValidY(){return validY;}

final Vector2f getValidPosition(){position.x = getValidX(); position.y = getValidY(); return position;}
}

I have a lot of classes like the one above which are being used as a data type to return specific information.
Logged
Xienen
Level 3
***


Greater Good Games


View Profile WWW
« Reply #5 on: May 30, 2013, 09:51:11 AM »

It seems that you're headed in the right direction with your code.  Not spreading out the hierarchy too flat, nor allowing it to build up to a massive sprawling hierarchy.  If you were all but done with the game, it wasn't a tiny puzzle game, and it was only 23 classes, I would start wondering how many lines of code are in those 23 classes.  It's very hard to balance, especially when you're new to programming or game development, but it seems like you're doing a good job.

Games are big code bases, there's just no way around it(except to build tiny puzzlers, but what fun is that?).
Logged

Chromanoid
Level 10
*****



View Profile
« Reply #6 on: May 30, 2013, 02:42:27 PM »

I would recommend to follow the Java coding style and use the outline view/code completion of your IDE to get an overview of a class instead of using these one line methods.  You should also use javadoc, then you can easily handle thousands of classes. Using reasonable package names will also help organizing your code.
Logged
nikki
Level 10
*****


View Profile Email
« Reply #7 on: May 30, 2013, 03:11:18 PM »

I would throw in some blank lines when it makes sense.
you know to improve readability

A thing that also helps, and I don't think you do this.
write to an API not an implementation.

write to an API not an implementation.







write to an API not an implementation.












I tried figuring out your code,
I see it has to do with collisions
I also see two constructors that look quite differnt.
I have only looked quickly but I think i don't undertand them.
Code:
               setCollision(false);   // ok initialize I assume ?
setCollisionX(false);  // huh
setCollisionY(false);  // huh
setPassable(true);     // based on what ?
setCollideType("null_affector"); // null_affector sounds groovy, wouldn't know what to do with it though
setValidX(position.x); // aha an argument get's put somewhere
setValidY(position.y); // another one, now we are talking.
this.position = new Vector2f(position);  // and now again ? the same


How are you using this class,
what methods get called by who and why ?

there are another few little things I don't get about your code:

boolean collision, collisionX, collisionY; // what is a collisionX, it's a bool so it is not the X position of the colliding/overlapping point or something.
what is a pDataType ?

it looks as if you don't validate the validX why don't you just use an x,y instead ?

and this thingie
when broken done

final Vector2f getValidPosition(){position.x = getValidX(); position.y = getValidY(); return position;}

so that says
Code:
final Vector2f getValidPosition()
{
    position.x = getValidX();
    position.y = getValidY();
    return position;
}


getValidX() returns what you have put in with setValidX //(position.x);
since that is just the same position you set in the constructor to be
this.position = new Vector2f(position);

you could just as well write your getValidPosition() as

Code:
final Vector2f getValidPosition()
{
    return position;
}
      
If you could define and name the actual intention of this example class that would be great, I mean I would like to write an API i'd like to see but I don't fully understand your intentionswith this code.

It looks the most like an eventData to send to me, but I think it is not. so yeah
but all in all I so think your code is growing out of control if I see this.

so take a trimmer and start cutting all the bush

oh and btw your not doing bad at all for a relative new programmer I think
Logged
Ralgarog
Level 0
***


View Profile
« Reply #8 on: May 30, 2013, 04:51:01 PM »

The collisionData class is a type that i defined to handle collision detection. It is mainly used in my Physics_Handler class seen below:

Code:
package slick;
import java.util.ArrayList;
import java.util.List;

import org.newdawn.slick.geom.Rectangle;
import org.newdawn.slick.geom.Vector2f;

public class Physics_Handler{
static Player play;

Physics_Handler(){

}

//Adds a player to the physics simulation. Currently not fully implemented.
static void AddPlayer(Player player) {play = player;}

//This is where the collision detection process takes place.
static public CollisionData Collision(Entity entity, Vector2f position, int delta){
Vector2f OldPosition = new Vector2f(entity.getPhysics().position);
Rectangle rect = new Rectangle(0,0,0,0);
rect = entity.getPhysics().boundary;

CollisionData collision = new CollisionData(position);

//Check for Collision along the Y Axis
rect.setLocation(new Vector2f(OldPosition.x, position.y));
for(Entity obj : Map.tileMap)
{
if(rect.intersects(obj.getBounds()) && obj.getBounds() != entity.getPhysics().Self()
                                                   && obj.getName() != "Player"){
collision.setActive(obj);
if(obj.isPassable()){

} else {
collision.setValidY(OldPosition.y);
collision.collideType = obj.getName();
collision.setCollision(true);
collision.setCollisionY(true);
rect.setLocation(collision.getValidPosition());
break;
}
} else {
collision.setValidY(position.y);
}

}

//Check for Collision along the X Axis
rect.setLocation(new Vector2f(position.x, OldPosition.y));
for(Entity obj : Map.tileMap)
{
if(rect.intersects(obj.getBounds()) && obj.getBounds() != entity.getPhysics().Self()
                                                   && obj.getName() != "Player"){
collision.setActive(obj);
if(obj.isPassable()){

} else {
collision.setValidX(OldPosition.x);
collision.setCollision(true);
collision.setCollisionX(true);
entity.setPosition(collision.getValidX(), entity.getPosition().y);
entity.getPhysics().setAcceleration("collide", 0 , 0);
break;
}
} else {
collision.setValidX(position.x);
}
}
return collision;
}


//Used for Calculation Gravity within the "physics simulation". Only partly implemented
static public void calcGrav(Entity entity, Vector2f pos, int delta){
Vector2f tempPosition = new Vector2f(pos);
tempPosition.y += delta * 0.3f;

if(Collision(entity, tempPosition, delta).getCollisionY()){
entity.setFallingState(false);
} else {
entity.setFallingState(true);
entity.setPosition(tempPosition.x, tempPosition.y);
Level.spr.setAction(1);
}
}


}

To make use of this code, i have another class PBody that handles all physics for any object that it is "attached to". For example: If i want to add collision detection or gravity to a block, or the ability for a block to detect collisions i add a PBody to it. The PBody class is listed below:

Code:
package slick;

import org.newdawn.slick.geom.Vector2f;
import org.newdawn.slick.geom.Rectangle;
import java.text.*;

public class PBody{

Rectangle boundary;
Vector2f position;
Vector2f acceleration;
Entity entity;
int width, height;
int weigth = 1;
float scale = 1;
String type;
String currentdirection;
CollisionData collide;
Timer timer;

final float ACCEL_MAX = 5;

public void setAcceleration(String x, float mod, int delta){
currentdirection = x;



switch(x)
{
case "left":
acceleration.x -= (acceleration.x >= -ACCEL_MAX) ?  mod*delta : 0;
this.TestCollision(new Vector2f(entity.getPosition().x + acceleration.x, entity.getPosition().y), delta);
break;
case "right":
acceleration.x += (acceleration.x <= ACCEL_MAX) ? mod*delta : 0;
this.TestCollision(new Vector2f(entity.getPosition().x + acceleration.x, entity.getPosition().y), delta);
break;
case "collide":
acceleration.x = 0;
return;
default:

acceleration.x += (acceleration.x > 0) ? -(delta * mod) : (acceleration.x < 0) ? delta * mod : 0;
if(acceleration.x < 1.0  && acceleration.x > -1.0)acceleration.x = 0;
this.TestCollision(new Vector2f(entity.getPosition().x + acceleration.x, entity.getPosition().y), delta);
}

if(acceleration.x >= 5)
acceleration.x = ACCEL_MAX;
else if(acceleration.x <= -5)
acceleration.x = -ACCEL_MAX;

timer.Update(delta);
}

final public Vector2f getAcceleration(){return acceleration;}
final public String getDirection(){return currentdirection;}
public PBody(Entity entity, Vector2f position, String type, int width, int height, float scale)
{
this.width = width;
this.height = height;
this.scale = scale;
this.entity = entity;
this.boundary = new Rectangle(position.x, position.y , width * scale, height * scale);
this.position = new Vector2f(position);
this.type = type;
this.collide = new CollisionData(position);
this.timer = new Timer();
acceleration = new Vector2f(0, 0);
currentdirection =  " ";
this.boundary.closed();
timer.enableTargetInterval(true);
timer.setTargetInterval(300.0f, 0);

}

public PBody(Vector2f position)
{
this.boundary = new Rectangle(position.x, position.y, 32, 32);
}

public CollisionData getCollisionInfo()
{
return collide;
}
public void SetBoundary(Vector2f pos)
{
this.position = pos;
this.boundary.setLocation(pos);
boundary.setCenterX(position.x + (boundary.getWidth() / 2));
boundary.setCenterY(position.y + (boundary.getHeight() / 2));
}

public void Update(Entity en)
{
this.position = en.getPosition();
boundary.setLocation(position);
boundary.setCenterX(position.x + (boundary.getWidth() / 2));
boundary.setCenterY(position.y + (boundary.getHeight() / 2));
this.boundary.setBounds(position.x, position.y, height * scale, height * scale);


entity.setPosition(this.position.x + acceleration.x, this.position.y + acceleration.y);

}

public Rectangle Self(){return boundary;}

public void TestCollision(Vector2f position, int delta)
{

DecimalFormat df = new DecimalFormat("#.#");
this.position.x = Float.parseFloat(df.format(this.position.x));
this.position.y = Float.parseFloat(df.format(this.position.y));

position.x = Float.parseFloat(df.format(position.x));
position.y = Float.parseFloat(df.format(position.y));

float oldX = this.position.x;
float oldY = this.position.y;
float tempX = position.x;
float tempY = position.y;

Vector2f tempPosition = new Vector2f(tempX, tempY);



collide = Physics_Handler.Collision(entity, tempPosition,delta);

if(tempY != oldY)
{
if(collide.getCollision())
{

entity.setPosition(collide.getValidPosition());

}
else
{

entity.setPosition(tempPosition);

}

}
else
{
return;
}

tempPosition.x = tempX;
tempPosition.y = tempY;

if(tempX!= oldX)
{
if(collide.getCollision())
{
entity.setPosition(collide.getValidPosition());

}
else
{
entity.setPosition(tempX, tempY);
}

}
else
return;

}

public CollisionData cfPlayer(int x, int y){
CollisionData collide = new CollisionData(position);
Rectangle border = new Rectangle(this.position.x, this.position.y, x + 1, y + 1);

if(border.intersects(Level.player.getBounds())){
collide.setActive(this.entity);
collide.setCollideType(this.entity.getName());
collide.setCollision(true);

}
return collide;
}

public boolean CheckForPlayer(int x, int y){
boolean result = false;
Rectangle border = new Rectangle(this.position.x, this.position.y, x + 1, y + 1);

if(border.intersects(Level.player.getBounds())) result = true;

return result;
}

public boolean CheckForPlayerY(float y){
boolean result = false;
Rectangle check =  new Rectangle (this.position.x + 8, this.position.y + y, 24, -y);

if(check.intersects(Level.player.getBounds()))
result = true;


return result;
}

public boolean CheckPosition(float x, float y)
{
boolean result = false;
Rectangle check = new Rectangle(x, y, 32, 32);

for(Entity e : Level.currentLevel.tileMap)
{
if(e == entity) continue;
if(check.intersects(e.getBounds()))
{
result = true;
break;
}
}


return result;
}

}

Effectively, within my code for my Item class for example all i need to do is create an instance of collisionData then i can do something like below:

Code:
CollisionData collision;
collision = this.getPhysics().cfPlayer(sprite.Render().getWidth(),   sprite.Render().getHeight());

if(collision.getCollision()){
 do something...
}

the "this" in the code is a particular instance of an implementation of an entity (Entity is made as an Interface. Object is an abstract implementation of Entity. Items is an extension (extends) of the Object abstract class) in this case the Items class. Since it is an entity it has a PBody attached to it. getPhysics() returns a handle to the specific instance of Items' specific instance of PBody.

My code is currently set up to test for collision on the Y axis then collision on the X axis. If there is a collision, collisionX and collisionY are set to true. However i have detection split between the X axis and the Y axis. It checks for Y collisions first, then X collisions. So i can easily implement realistic slope traversal or have a block push me of something without any issues.

What you see in that constructor spinet is me setting everything to a default state. By default, the object will be considered passable because by default no entity (something containing a PBody) is passed to the class.

pDataType is the type of PBody, be it static, dynamic or hybrid. I have this difference set for updating purposes. It currently does not have a use but it is there for when i update the order that collisions are handled by in the future.

As for the Valid X and the Valid Y, that is used for the collision detection i described earlier. Since i detect X and Y collisions separate, it allows my to continue moving along my X axis if my Y axis is blocked. I.E: Valid X and Valid Y are the last know X and Y values where collision have not taken place.

GetValidPosition does return what was set by valid position, however as i mentioned earlier X and Y collisions are tested separately, therefore i cannot just pass it the position that i went into the code using.

The code (CollisionData) was only posted here as an example of a helper class (in this case i guess a helper type) that i made so that i can do things like what was listed earlier in this post.

I apologize about the wall of text, and i greatly appreciate your feedback thus far.
Logged
Pages: [1]
Print
Jump to:  

Theme orange-lt created by panic