An elevator has four states: (1) moving; (2) stationary with doors opening; (3) stationary with doors open; (4) stationary with doors closing; With your "moveTo" field, the current state can be determined by the following: (1) getY() != moveTo && leftDoor.getX() == getX()+2; (2) getY() == moveTo && leftDoor.getX() > getX()-18; (3) getY() == moveTo && leftDoor.getX() == getX()-18; (4) getY() != moveTo && leftDoor.getX() != getX()+2 (my signs may be off as I do not know exactly how your images are constructed to work together) If left and right doors have symmetrically opposing motion, checking right door is unnecessary (move both on same left condition). From the above a simple format would be (with 1st conditions being "A" and the second ones "B"):

I don't think you interpreted the code correctly, I made it easier to read. The controller is 34(Length) by 1 (height), so it is suppose to open when it is in the middle of the desired elevator location. When it is above or under it it should close the doors. So it should be closing constantly when it hasn't stopped, so thats why there is the if statements so they dont open to much or close to much. In my mind it should always be working... Closing when it has not reached its destination(but is touching elevator doors, also wont close when doors already are closed) and opening when it has reached its destination I also used the a range in the else if because it should close when it is going up or down. int moveTo=538; public void act() { checkMouse(); active(); } public Controller() { this.setImage("Controller.png"); } public void checkMouse() { MouseInfo mi = Greenfoot.getMouseInfo(); if (mi != null) { int buttonNumber = mi.getButton(); if(buttonNumber==3 || buttonNumber==1) { if (mi.getX() > this.getX()-20 && mi.getX() < this.getX()+20) move(); } } } public void move() { MouseInfo mi = Greenfoot.getMouseInfo(); if(mi.getY() > (100-20) && mi.getY() < (100+20)) moveTo=100; if(mi.getY() > (173-20) && mi.getY() < (173+20)) moveTo=173; if(mi.getY() > (246-20) && mi.getY() < (246+20)) moveTo=246; if(mi.getY() > (319-20) && mi.getY() < (319+20)) moveTo=319; if(mi.getY() > (392-20) && mi.getY() < (392+20)) moveTo=392; if(mi.getY() > (465-20) && mi.getY() < (465+20)) moveTo=465; if(mi.getY() > (538-20) && mi.getY() < (538+20)) moveTo=538; } public void active() { if(moveTo != this.getY()) { if(moveTo < this.getY()) { setLocation(this.getX(), this.getY()-1); } if(moveTo > this.getY()) { setLocation(this.getX(), this.getY()+1); } } int gety=this.getY(); if (moveTo == gety) { LeftDoor leftDoor = (LeftDoor) getIntersectingObjects(LeftDoor.class).get(0); RightDoor rightDoor = (RightDoor) getIntersectingObjects(RightDoor.class).get(0); if(leftDoor.getX() > getX()-18)//moves door when elevator is stationary leftDoor.setLocation(leftDoor.getX()-2, leftDoor.getY()); else if(rightDoor.getX() < getX()+18)//moves door when elevator is stationary rightDoor.setLocation(rightDoor.getX()+2, rightDoor.getY()); } else if (gety > (moveTo-15) && gety < (moveTo+15) && moveTo != gety) { LeftDoor leftDoor = (LeftDoor) getIntersectingObjects(LeftDoor.class).get(0); RightDoor rightDoor = (RightDoor) getIntersectingObjects(RightDoor.class).get(0); if(leftDoor.getX() < getX()+18)//moves doors when new location is selected leftDoor.setLocation(leftDoor.getX()+2, leftDoor.getY()); else if(rightDoor.getX() > getX()-18)//moves doors when new location is selected rightDoor.setLocation(rightDoor.getX()-2, rightDoor.getY()); } else; }

I think the "else if ..." line in active should just be "else".

This is what happens when I get rid of else in active's else if java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0 at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) at java.base/java.util.Objects.checkIndex(Objects.java:372) at java.base/java.util.ArrayList.get(ArrayList.java:458) at Controller.active(Controller.java:78) at Controller.act(Controller.java:19) at greenfoot.core.Simulation.actActor(Simulation.java:567) at greenfoot.core.Simulation.runOneLoop(Simulation.java:530) at greenfoot.core.Simulation.runContent(Simulation.java:193) at greenfoot.core.Simulation.run(Simulation.java:183)

Honestly, I really think that this code just needs a rework. There is way more going on then there needs to be. Instead of relying on all 3 parts being exactly where they need to be, I would make a class with a permanant location along with the doors as permanant properties.