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

Arrow hit limit #45

Closed
wants to merge 4 commits into from
Closed

Arrow hit limit #45

wants to merge 4 commits into from

Conversation

afyber
Copy link
Member

@afyber afyber commented Mar 23, 2019

Adds a limit to the number of entities an arrow can hit before disappearing. Also reduces the damage for hits after the first.

Copy link
Member

@chrisj42 chrisj42 left a comment

Choose a reason for hiding this comment

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

So I think you'll find, if you test it, that this does not work as intended. The tick method is not called every time a separate mob is hit or something, it is called every frame, and an arrow will almost certainly be in contact with a mob for multiple frames at a time. This will increment mobsHit much faster than the number of mobs the arrow actually damages, or more precisely, the number of times the arrow hurts a mob.

The reason mobs are not damaged every tick is because they have an internal hurt cooldown.
What you want to do, then, is make a little refactor of the Mob hurt methods and doHurt method to return whether the mob was actually hurt or not. Then you can use this value to determine if mobsHit should be incremented or not. Also I think 2 mobs is too low, try 3 or 4.

@chrisj42 chrisj42 added the Improvement Ideas for changes to features or content. label Mar 23, 2019
@afyber
Copy link
Member Author

afyber commented Mar 23, 2019

like that?

@chrisj42
Copy link
Member

Did you test it? think about what happens on a server. If you return false from doHurt for all remote players, then the arrow will never actually go away if it just hits players. But if you return true, then it will damage every frame.
So you see, there's actually a bigger problem here. Clients should not be dealing with damage to themselves, because then it messes up the server, and besides, it also opens up the ability to cheat.
And then, if you look at arrows, you see that they are updated on both the client and server, and that doesn't make total sense either. After all, then the client would track all arrow hits, and would remove the arrow, but the server would not remove it, and would continue until it hit 4 non-player mobs, resulting in apparent "ghost damage" in the client side.
Ideally, they would all be updated on the server, and the only things updated on the client are those that the server doesn't track, i.e. the particle subclasses. However, the player is updated partially on the client to mitigate lag.

So essentially, I can't accept this until you have a way of checking if the player actually got hurt on the server, so the ghost damage doesn't happen.

@afyber afyber closed this Mar 24, 2019
@afyber afyber deleted the arrow-hit-limit branch March 24, 2019 03:06
@afyber afyber restored the arrow-hit-limit branch March 24, 2019 03:06
@afyber afyber deleted the arrow-hit-limit branch March 30, 2019 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Ideas for changes to features or content.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants