Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Player can move units #62

Merged
merged 20 commits into from Mar 31, 2015
Merged

Conversation

stefanhendriks
Copy link
Contributor

Fixes #38

  • player can select unit
  • player can order unit to move
  • unit can move pixel by pixel
  • unit can have their own move speed
  • player can move only its own units
  • unit will not stop half-way (pixel based) when ordered to move somewhere else

demo at: https://www.youtube.com/watch?v=lJy1o8QYEhg&feature=youtu.be

After review:

  • fix bug when two units move towards each other taking up the same cell (??)
  • use polymorphism for Mouse class instead of having booleans. (ie, replace procedural polymorphism with inheritance) (demo at: https://www.youtube.com/watch?v=IcIq43m31sE)
  • write/fix tests which are now disabled/removed by refactoring of Mouse
  • When clicking on another unit - while a unit is already selected - doesn't change the select
  • Make moveSpeed configurable for unit instead of randomized in constructor

…next cell to move to is and jump to that. When it is occupied, then stop moving.
…his fixes any odd behavior as in trying to move left-down instead of diagonally (which may fit, as opposed to left-down, because at the left you might have another unit... - which causes an exception to be thrown when moving onto that)
@stefanhendriks
Copy link
Contributor Author

just added - "player can move only its own units"

… unit does not end up half on a cell when turning around. (make sure unit stays on one cell!)
@@ -12,12 +14,18 @@
*/
public class Mouse {

private Player playerControlled;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

playerControlled sounds like a boolean value. I like the name controllingPlayer better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@arjenvanderende
Copy link
Collaborator

When clicking on another unit - while a unit is already selected - doesn't change the select, but moves the selected unit instead. This seems counter-intuitive. Is that supposed to happen?

@arjenvanderende
Copy link
Collaborator

I'm noticing a z-index issue with the unit selection box: when a unit is selected and it drives diagonally past another unit, then the selection box is draw underneath the other unit. This seems logical: we draw the unit and the selection box in one pass, but they should probably be separate. Either we need to draw selection boxes after all units are rendered, or we need to change the draw order of units so it draws selected units last. I prefer the 1st option.

Should we move this into another issue?

@Override
public void update(float deltaInMs) {
this.fadingSelection.update(deltaInMs);
if (shouldBeSomewhereElse()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol, I really like this method name 😄

@arjenvanderende
Copy link
Collaborator

I crashed the game! 😈

It's not that hard to reproduce: place two unit opposite of eachother like this:
screen shot 2015-03-16 at 21 45 23

Then select one unit and move it past the other unit. Quickly deselect the unit and select the other one. Then move that unit in the direction of the other one. Putting the unit further apart makes it easier to do so. If the timings of both units are different (meaning unit 1 is in the center of a cell, while unit 2 is moving between two cells) then they will be allowed to occupy the same cell. After that the game crashes with the following stack trace:
screen shot 2015-03-16 at 21 44 05

Probably a unit is allowed to move to the next cell, even though it doesn't occupy it yet.

@stefanhendriks
Copy link
Contributor Author

@arjenvanderende created #65

@stefanhendriks
Copy link
Contributor Author

Mouse refactoring is done, still need to add tests, but it feels much better! (it is actually quite easy to add changes to the behaviors now, like for example see what you're placing in debug mode) -
image

@stefanhendriks
Copy link
Contributor Author

showing off the refactored mouse states at -> https://www.youtube.com/watch?v=IcIq43m31sE

the only thing you can 'see' is that the mouse no longer snaps to the grid and that you can see what unit/structure is being placed.

@stefanhendriks
Copy link
Contributor Author

Went a bit out of scope, but hey its fun! see: https://www.youtube.com/watch?v=tfRu9bwIoVQ&feature=youtu.be

@stefanhendriks
Copy link
Contributor Author

Bug is fixed!

stefanhendriks added a commit that referenced this pull request Mar 31, 2015
@stefanhendriks stefanhendriks merged commit 047f8bb into master Mar 31, 2015
@stefanhendriks stefanhendriks deleted the feature/38-player-can-move-units branch March 31, 2015 20:51
@stefanhendriks
Copy link
Contributor Author

After watching DHH's talk (https://www.youtube.com/watch?v=9LfmrkyP81M) - I think we should move forward (value) and write tests later. As in, it is a waste to keep this PR open (it is the only active branch, so can be master as well).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Player can move units
2 participants