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

Update 2_create_player_character.rst #57

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ specific animations for the character for each direction angle:
The ``Speak`` animations are optional and only required if your game needs
them.

For each direction angle, put the name of the matching animation (with the name
specified in "Adding a walkcycle" above) in the "Animation" field, and choose
For each direction angle, each slot needs to contain a ``ESCAnimationName`` resource. Put the name of the matching walking animation (with the name specified in "Adding a walkcycle" above) in the "Animation" field, and choose
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
For each direction angle, each slot needs to contain a ``ESCAnimationName`` resource. Put the name of the matching walking animation (with the name specified in "Adding a walkcycle" above) in the "Animation" field, and choose
Each slot for the above animations needs to contain a ``ESCAnimationName`` resource,
corresponding to each direction angle. Put the name of the matching animations (using
the names specified in "Adding a walkcycle" above) in their respective "Animation" fields,
and choose whether each animation should be played mirrored by selecting the
"Mirrored" checkbox.

Copy link
Author

Choose a reason for hiding this comment

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

The escanimationname resource should go before anything else, otherwise the user following the tutorial won't understand what to use until they scroll below the images that use the resource.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand what your argument is: The suggested change I made follows your change, but uses slightly different verbiage. ESCAnimationName still appears in exactly the same place you had it.

Copy link
Author

Choose a reason for hiding this comment

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

Sure thing, I'm new to pull requests so I wasn't entirely sure what was approved or not, sorry!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, no worries. Yeah, GitHub's PR interface takes a little getting used to. So far, nothing's been "approved", but I have requested a change to be made (above) before I decide on whether to accept the PR.

Note that, for this project, approval requires "acceptance" from at least two of the devs.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh thanks for the explanation!

Copy link
Collaborator

@balloonpopper balloonpopper May 15, 2022

Choose a reason for hiding this comment

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

I'd actually like to overhaul this part of the documentation as I think we could explain the world angle to ESCAnimation resource to actual animation mechanism a lot better. Happy to run with changing this section in the interim.

whether that animation should be played mirrored by selecting the "Mirrored"
checkbox.
Comment on lines 208 to 209
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
whether that animation should be played mirrored by selecting the "Mirrored"
checkbox.


Expand All @@ -214,7 +213,6 @@ These are the the first 3 settings for the ``Directions`` parameter:
.. image:: img/character_create_animation_directions.png
:alt: The settings required for the Direction parameter

Each slot needs to contain a ``ESCAnimationName`` resource.
Try to set up the ``Directions`` and ``Idle`` animations by yourself.
Don't forget to click ``Mirrored on`` when the image is facing the
opposite direction (for instance we have a left animation and
Expand Down