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

Fix excess projectiles in the same tick impacting entities that die that tick #6532

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

lL1l1
Copy link
Contributor

@lL1l1 lL1l1 commented Nov 12, 2024

Issue

If multiple projectiles hit a target in the same tick, they will all impact it regardless of the target's HP.
For example, 6 TML (36000 damage) placed in a line will all impact a 13000 HP shield if they hit on the same tick. In general, any line/concave formation of units could be susceptible to this bug.

This is because collisions are checked for all projectiles before any impact scripts are run, so the usual OnCollisionCheck if self.DisallowCollisions then return false end doesn't work if DisallowCollisions is adjusted due to damage taken from impacts.

Description of the proposed changes

  • Check DisallowCollisions in the OnImpact function of the projectile, so that projectiles have an up-to-date status on the entity's collision as HP gets adjusted due to previous impacts.
  • Add the DisallowCollisions boolean to shields because:
    1. _IsUp would be an extra check for all projectiles
    2. _IsUp is adjusted by state changes, which are forked threads that happen after all impact scripts are run.

Testing done on the proposed changes

Use a logging statement to see how many projectiles impact a unit or shield and how many impact the terrain. The projectiles are from 15 TML stacked on top of each other, although realistic scenarios like 6 TML in a line can also impact a shield in the same tick.
The logging statement shows that just enough projectiles to kill the unit/shield hit it, and the rest passed on to hit the terrain behind/under it.

Tested with a shield, a Ythotha (unit with death animation), Percival (standard unit), and a Tempest (sinking animation unit). Before the changes, all 15 TML would impact the target, and after the changes only the necessary amount impact the target.

Checklist

  • Changes are annotated, including comments where useful
  • Changes are documented in the changelog for the next game version

for example: 6 TML all impacting 1 shield on 1 tick, despite the shield only having HP for 3 TML
@lL1l1 lL1l1 added type: bug area: sim Area that is affected by the Simulation of the Game labels Nov 12, 2024
@lL1l1 lL1l1 marked this pull request as ready for review November 12, 2024 12:13
@Garanas
Copy link
Member

Garanas commented Nov 12, 2024

This is tricky. I do not perceive this to be that big of an issue. I am not sure if it is even a bug. It's definitely a balance change - various units that fire multiple projectiles the same tick may now be more viable to take out shields and whatever is underneath it.

I think this requires more discussion about whether it is a bug, and if it is, whether it is worth introducing the additional logic to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: sim Area that is affected by the Simulation of the Game type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants