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

Add Run Energy depleting/restoring #423

Open
wants to merge 5 commits into
base: kotlin-experiments
Choose a base branch
from
Open

Add Run Energy depleting/restoring #423

wants to merge 5 commits into from

Conversation

Tomm0017
Copy link

@Tomm0017 Tomm0017 commented Apr 25, 2019

Addresses Issue #87 in which run energy depletion and restoring is requested

@Tomm0017 Tomm0017 changed the title run_energy Implement Run Energy depleting/restoring Apr 25, 2019
@codecov-io
Copy link

codecov-io commented Apr 25, 2019

Codecov Report

Merging #423 into kotlin-experiments will decrease coverage by 0.03%.
The diff coverage is 5.26%.

Impacted file tree graph

@@                   Coverage Diff                    @@
##             kotlin-experiments     #423      +/-   ##
========================================================
- Coverage                 23.23%   23.19%   -0.04%     
  Complexity                  811      811              
========================================================
  Files                       643      643              
  Lines                     11123    11139      +16     
  Branches                   1632     1634       +2     
========================================================
  Hits                       2584     2584              
- Misses                     8251     8267      +16     
  Partials                    288      288
Impacted Files Coverage Δ Complexity Δ
...o/game/sync/task/PrePlayerSynchronizationTask.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...main/java/org/apollo/game/model/entity/Player.java 36.36% <7.69%> (-1.65%) 24 <0> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd06a69...ec165b2. Read the comment docs.

@Tomm0017 Tomm0017 changed the title Implement Run Energy depleting/restoring Add Run Energy depleting/restoring Apr 25, 2019
@@ -58,6 +60,15 @@ public void run() {
Position old = player.getPosition();
player.getWalkingQueue().pulse();

if (player.getSecondDirection() != Direction.NONE) {
Copy link
Contributor

@garyttierney garyttierney Apr 26, 2019

Choose a reason for hiding this comment

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

This isn't right. We should only decrement the run energy when the player actually moves (see WalkingQueue). The direction could be set but the collision maps said this is an invalid move.

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't fond of the condition, but it seemed to be the best fitting choice for what needed to be done. Sorry if I'm wrong here and just wasting your time, but I believe the direction is only set if the collision map allows the movement (see https://github.com/apollo-rsps/apollo/blob/kotlin-experiments/game/src/main/java/org/apollo/game/model/entity/WalkingQueue.java#L156).

Also, am I wrong to assume that if I use WalkingQueue.isRunning() & WalkingQueue.size(), then I would run into the issue that you warned about? I believe when adding steps into the walking queue, it won't check for collision.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I need to re-look at the assignment of secondDirection, you may be right there.

Copy link
Member

Choose a reason for hiding this comment

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

@garyttierney any update on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

The code is fine as is, and what we expect. That's the only assignment of secondDirection and the point @Tomm0017 made is valid.

* @param weight the amount of weight to be taken into account when
* determining how much run energy will be depleted.
*/
public void decrementRunEnergy(double weight) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Major- any thoughts on this being in core? Seems like it might be the responsibility of a plugin, but I'm not sure.

Copy link
Member

Choose a reason for hiding this comment

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

I think core is correct

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.

4 participants