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

Character Animation #669

Merged
merged 6 commits into from
Aug 23, 2016
Merged

Conversation

Raketfart
Copy link
Contributor

Added a CharacterAnimation class, and hooked it up to Character and CharacterSpriteController.
The helmet is still a seperate sprite, so only the character is animated.
See issue #142

@@ -57,10 +57,24 @@ public void OnCharacterCreated(Character c)
char_go.transform.SetParent(characterParent.transform, true);

SpriteRenderer sr = char_go.AddComponent<SpriteRenderer>();
sr.sprite = SpriteManager.current.GetSprite("Character", "p2_front");
//sr.sprite = SpriteManager.current.GetSprite("Character", "p2_front");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not be commented out code

@Grenkin1988
Copy link
Contributor

You should run StyleCop, looks like there are some issues. (For example to many empty lines)

@Raketfart
Copy link
Contributor Author

Commented line is removed, and I ran StyleCop on CharacterAnimation.
There are MANY stylecop issues in Character and CharacterSpriteController.
Should I clean up those too? Or will that mess too much with other pull requests?

@Fogsight
Copy link
Contributor

It is probably the best for clean up to be performed separately, it can be snuck through when nobody working on the file, harmlessly.

@@ -329,7 +338,7 @@ private bool CheckForJobMaterials()
// Walk towards a tile containing the required goods.
Debug.Log("Walk to the stuff");
Debug.Log(myJob.canTakeFromStockpile);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Theres a handful of lines in this file with trailing white space. Could you fix that? :)

@Raketfart
Copy link
Contributor Author

@Mizipzor I removed the whitespace now.

public bool IsWalking;

// 0=north, 1=east, 2=south, 3=west
public int Facing;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should use an enum here. It would make things look more clear.

@alexanderfast
Copy link
Collaborator

This is "just" a refactor, right? The character should be equally animated before and after this is merged?

@Raketfart
Copy link
Contributor Author

@Mizipzor Uhm, what do you mean? The character wasn't animated before
This PR makes his feet move and his facing match his direction.

@Raketfart
Copy link
Contributor Author

This is what it looks like:
animation

@Tranberry
Copy link
Contributor

should work with #684

@alexanderfast
Copy link
Collaborator

I couldve sworn my character was animated before I tested this. I might be crazy, let me have another look.

@alexanderfast
Copy link
Collaborator

Nope, he wasnt. Either I was running with your changes when I thought I wasnt, or I'm actually crazy.

Good work on the enum though, I would've asked for that as well. :)

Good job!

@alexanderfast alexanderfast merged commit 8d9cbaf into TeamPorcupine:master Aug 23, 2016
@Raketfart Raketfart deleted the Animation-test branch August 23, 2016 19:47
Tritaris pushed a commit to Tritaris/ProjectPorcupine that referenced this pull request Aug 23, 2016
* Fixed position of sprite tiles. Now Y positions are counted from the bottom up.

* Added CharacterAnimation class and hooked it up to character and characterspritecontroller

* Character animation working

* Ran stylecop on CharacterAnimation

* Removed whitespace

* Changed facing from int to Enum
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.

6 participants