Refactoring Help: States as Entities?


(HeraldR) #1

I’ve been working on a volleyball game off and on for ages. Last week I decided to get back on it and have been trying to refactor the code. I’ve learned a lot since I last made major changes to it, but not enough. While the code is definitely better, it’s still pretty convoluted. I know what I would like to do with refactoring but I am not sure how exactly to do it in flashpunk (or any language for that matter).

I have a god player class (well, 2 of them actually, but part of the refactoring is to get rid of the other one) that is currently down to 1500 lines from 2000. It’s actually divided into states using the state of the player and the state of the ball. For instance: `if (ball.isTeamABall() == isTeamA) {

			offense();
		} else {
			defense();
		}`

and then farther down:

	private function defense():void 
	{	
		if (_state == "knockeddown") {
			knockeddown();
			return;
		}
		if (ball.isState("served") && (_state == "served") ) {
			served();
			return;
		}
		if (ball.isState("blocked")) {
			// divetowardstarget();
			return;
		}
		if (ball.isState("spiked")) {
			// if (!blocking) { getclosestanddive(); }
			return;
		}
		if (_state == "blocking") {
			blocking();
			return;
		}
		if (!target.isSameSideAsAthlete(x, y)) {
			// run to defensive positions();
			return;
		}
	}

You get the idea. I would like to move some of the states such as blocking/spiking/serving into their own classes, but I don’t know how to do that. Do I just make classes named that and extend entity? Then do I just create new blocking/spiking/serving entities and add them to the Player? Each of the entities would need to have movement and animation capabilities to be worth anything wouldn’t they? I just don’t understand what people mean when they say to move stuff into its own class.

Any suggestions would be greatly appreciated!


(Ultima2876) #2

An example would be to have a variable in your player called _currentState:

private var _currentState: IPlayerBehaviour;

IPlayerBehaviour here represents an interface. An interface is a special type of class that you ‘implement’ (similar to extending a class) to say that a bunch of different classes will definitely perform some function. For example, in our case, we want a bunch of classes that have an update function. What the update function does can differ, but we definitely want one in each different Behaviour class. So we create our interface (in FlashDevelop, right click a folder and add > new interface) and add our update function prototype:

package src.entities.player.behaviours
{
	
	/**
	 * ...
	 * @author 
	 */
	public interface IPlayerBehaviour 
	{
		function update(player: Player): void;
	}
	
}

This essentially says that any class that ‘implements’ IPlayerBehaviour will have that update function that takes the Player class as an argument. This means you could have as many different behaviours as you need (PlayerServedBehaviour, PlayerSpikedBehaviour etc) and they can all have different update functions, then they will all be ‘compatible’ with our player’s _currentState variable (because it is of type IPlayerBehaviour). So lets create a basic behaviour (right click folder, add > new class, and fill in the interface in the ‘interfaces’ box):

package src.entities.player.behaviours
{
	/**
	 * ...
	 * @author ...
	 */
	public class PlayerServedBehaviour implements IPlayerBehaviour
	{
		
		public function PlayerServedBehaviour() 
		{
			
		}
		
		public function update(player: Player): void
		{
			//logic for this state goes here
		}
		
	}

}

Now you have a separate class for that behaviour. You can go ahead and create loads of classes for all the different behaviours. The final thing to do is make the Player class actually use them. I like the idea of using a state map for this - basically, you’ll have a Dictionary of behaviour classes stored in the Player and referenced by names (“serving” will pull out a PlayerServingBehaviour instance, for example), then you set _currentState to that. Finally, in the player’s update, all you need to do is use _currentState.update(this); to call the current behaviour’s update function. The power here lies in the fact that it’s completely flexible; it doesn’t matter what behaviour class is actually in _currentState, it will just call the update function then the class itself handles all the logic (keeping your Player class clean and tidy, and much easier to update).

So at the top of our Player class:

private var _behaviourList: Dictionary;
private var _currentState: IPlayerBehaviour;
private var _stateName: String;

In our player constructor we want to set up the Dictionary to add all the states that exist and link them to a string name:

_behaviourList["blocked"] = new PlayerBlockedBehaviour;
_behaviourList["spiked"] = new PlayerSpikedBehaviour;
_behaviourList["blocking"] = new PlayerBlockingBehaviour;
//set your starting state here
_stateName = "blah";
_currentState = _behaviourList[_stateName];

Finally, in the Player’s update function we add the magic to make it all work:

//update state to match the _stateName.
_currentState = _behaviourList[_stateName];
//run the current state update
if(_currentState != null) _currentState.update(this);

We pass in the ‘this’ reference to our Player class to make sure we can easily change the Player’s variables (x/y etc) in the behaviour class.

Then all you need to do is change the _stateName whenever you want the player’s behaviour to change. Perhaps make it public or give it a setter function, so that other classes can change it (or indeed, behaviour classes can change it as appropriate!). This kind of technique is incredibly flexible and powerful, and once you get used to thinking in terms of interfaces, abstraction etc it can really change the way you think about programming.


(HeraldR) #3

I can’t seem to get it to work. I like that it moves a lot of the code out of the Athlete class, but I don’t like how the code in the class looks uglier with athlete.x athlete.y athlete.playanimation athlete.inAir = 0. I think I might still use it, but I also ran into another problem. Take a look at the AthleteServingBehavior class (link here if this is too ugly: http://pastebin.com/e9ZFpDRa):

	public function AthleteServingBehavior() 
	{
		
	}
	
	public function update(athlete:Athlete):void 
	{
		if (athlete.inAir) {
			if (athlete.collide("volleyball", athlete.x, athlete.y)) {
				if (Input.pressed(Key.Z)) {
					// if in the air, colliding with ball, and button pressed: serve
					athlete.ball.prepareServe();
					GC.TARGET.x = Input.mouseX;
					GC.TARGET.y = Input.mouseY;
					GC.TARGET.setTarget = false;
					athlete._state = "served";
					athlete.sfxBump1.play();
					athlete.playAnimation("servespike");
				}
			}
		}
		
		if (!athlete.ball.isAirBorne()) {
			athlete.playAnimation("servestand");
			if (Input.pressed(Key.Z)) {
				// if ball isn't in the air and button is pressed: throw ball up
				GC.lastTouch = 1;
				athlete._outOfBounds = true;
				athlete.ball.serveJump();
				athlete.playAnimation("serve");
			}
		}
		
		if ((!athlete.inAir) && athlete.ball.getY() < 230) {
			// if ball is above a certain and athlete is not in air: jump
			athlete.playAnimation("servejump");
			athlete.addTween(new Alarm(2, moveIntoAir, 2), true);  // jump at end of servejump animation
			GC.SPIKEShadow.visible = true;
			GC.SPIKEShadow.x = athlete.x;
			GC.SPIKEShadow.y = athlete.y + athlete._playerShadowOffset;
			athlete._spikeShadow = true;
		}
		
		if(athlete.inAir) { move(athlete); }
	}
	
	// while in the air
	private function move(athlete:Athlete):void {
		var temp:Number;
		temp = ((GC.SPIKEShadow.y - athlete._jumpHeight) - athlete.y) * .1;
		athlete.y += temp;
		if (temp > -.5) {
			athlete._applyGravity = true;
			athlete._state = "served";
		}
	}
	
	// called at end of serve throw animation
	private function moveIntoAir(athlete:Athlete):void {
		athlete.inAir = true;
		GC.SPIKEShadow.visible = true;
		GC.SPIKEShadow.x = athlete.x;
		GC.SPIKEShadow.y = athlete.y + athlete._playerShadowOffset;
		athlete._spikeShadow = true;
		GC.TARGET.setTarget = true;
		athlete.sfxJump4.play();
		athlete.playAnimation("prespike");
		athlete.sm_jump.play("", false);
		athlete._applyGravity = false;
	}
}

The function moveIntoAir is what the callback function for the animation “servejump” normally does. I would like to move it into the behavior class so that I don’t have to look into the Athlete class for animation callbacks. I added an alarm Tween to try to get around this, but since the callback function is in the AthleteServingBehavior class and not the Athlete class, the function does not have access to the athlete. It seems that callback functions can’t have parameters. I don’t know how to get the callback function in the AthleteServingBehavior class to see the athlete without actually moving it back into the Athlete class.

One thing that looks particularly ugly is accessing things like the ball object the player has. I was working on getting rid of all of the GC references (and they will soon be gone), but now without the “GC” I still have to do athlete.ball.someFunction() etc. or instead of GC.Spikeshadow.visible it will be athlete.spikeshadow.visible.

Am I doing this right or is this completely wrong?


(Ultima2876) #4

You’re doing it right; one of the downsides to this approach (and object oriented programming in general) is that you will have a lot of x.y.z() going on. There’s not a whole lot you can do to avoid that and really it eventually just becomes a second nature part of the language (and actually in many cases makes your code easier to read as it explicitly says where these variables reside every time - eg in the ball class, or in the player class).

If you would definitely like to ‘work around’ this (which I absolutely would not recommend!), you could make copies of these important variables in your behaviour classes (eg x, y, inAir etc) and make sure they’re kept in sync with the player variables, then use those. This will eliminate all the athete.x stuff, but leaves your code more bloated and more prone to human error (not remembering to update a specific variable, mainly) leading to the potential for future headaches. If you wanted to do this, I’d say the best method is to have a separate ‘updateProperties’ method that handles this, then make sure that’s specified in your interface and called from the Player update as well as the regular update function.

With regards to the callback function, easy solution there; store your reference to the athlete as part of your behaviour class. At the top of the behaviour class simply add a variable:

private var _athlete: Athlete;

Then simply set that at the top of your update function ("_athlete = athlete;") (or better, in the constructor for your behaviour classes, and remove the update function’s Athlete argument altogether). Then you can use the class-stored reference to the athlete in your callback function. This also has the added advantage of removing the need to pass your athlete reference around to the other functions as they can always access the class variable (move).


(HeraldR) #5

Thank you! That got around the issue of the callback function. When refactoring my code I was trying to get rid of most of the x.y.z() occurrences, but being able to put the code for each state in one place is going to make it much easier to keep track of going forward. This Interface idea seems to solve my concern of making states as entities and how that would suck up resources. My hesitation with the z.y.z() besides it looking ungainly is just my concern with not being able to control what a state can modify, but this actually should be pretty easy to track down bugs through that since I’ll know what state is executing when errors occur and just look in there. It really was a pain having to scroll from my control code at the top, down to the state code in the middle of the file, down to the animation code at the bottom, up to the callback code slightly higher, down to the bottom where the movement code was. It was great having someone tell me what direction to go in when I was so unsure of every direction.

I plan to post the before and after of my refactored code in a couple of weeks (or sooner if it gets done sooner, but I’ve learned that even though I’m making good progress each day there are always things that take longer than I expected).


(Ultima2876) #6

Awesome. Programming is always a learning experience; part of what makes it so rewarding is that you’re constantly learning and improving! Keep us updated on how it’s going - we’re always happy to help here and answer questions!


(der_r) #7

Just thought I’d throw this in here. :smiley: Kyle Pulver has a nice implementation on his blog: http://kpulv.com/125/AS3_State_Machine/


(HeraldR) #8

That Kyle Pulver link is interesting. The last image with the different states and the arrows going in every direction is a little scary.

I’m refactoring with state machines now, but over the last few days I’ve been reading about Behavior Trees (found out about them from googling behaviors since that is what I am calling the states in my program). They seem like they would get around two big issues with state machines. The first problem is the one in that picture on kyle’s site, a jumble of transitions would be neatly organized. The other problem I think it would fix would be that of tracking down bugs caused by sharing object values among the states. Because you can reuse individual nodes and potentially keep them smaller, it should be easier to winnow the location of a bug down to a small area. This may just be wishful, grass is always greener, thinking, so I am still working on states, but I think I might try to get a state machine working afterwards just for practice and comparison.

One question, is it possible in any programming language to log function calls or variables throughout the program? Like, if a variable changes value I could log it with the id of the object calling it (and the game time, but that would be easily done just by logging the game timer each frame)? Or perhap logging every function call (and what object called it). That would probably be the most useful for a behavior tree, because I could follow the progression of the tree. Does action script 3 have something like that? Does any language?

Links for behavior trees: http://www.altdevblogaday.com/2011/02/24/introduction-to-behavior-trees/ http://aigamedev.com/open/article/behavior-trees-part1/


(Zachary Lewis) #9

For debugging with FlashDevelop, look into breakpoints and watch variables.

You should be able to set a breakpoint at any line in your code, then once the program flow reaches that point, you should be able to see every property of all objects that exist in memory.