unitystation / unitystation

The original unitystation
https://unitystation.org
GNU Affero General Public License v3.0
704 stars 648 forks source link

fix pkas #4844

Closed yoysam closed 3 years ago

yoysam commented 3 years ago

Purpose

fixes issue #4791

Notes:

added a new damage type and allowed it to do less when in a high-pressure environment"

yoysam commented 3 years ago

fixed

corp-0 commented 3 years ago

so what is your idea with this new damage type?

yoysam commented 3 years ago

this was the main idea of how to fix it this damage type if for the pkas or any other gun that would need to have its damage ajusted

corp-0 commented 3 years ago

could you make it work with attack type and keep brute damage as the damage type?

ReiderJack commented 3 years ago

You actually can just create another scripts for dealing damage which will be checking atmos pressure on hit and attach those scripts to PKA and plasma cutter bullets instead of default dealing damage scripts.

yoysam commented 3 years ago

i have no clue how i would do that since I don't fully understand the damage calulations

yoysam commented 3 years ago

so just right a whole other script with inhertience?

corp-0 commented 3 years ago

see Armor.cs. In short, when you perform an attack you send an attack type to the victim and the victim reduces the incoming damage by the armor they have against that particular attack type, then the victim is damaged by the reduced amount and its kept track in a particular damage type.

From what I discussed with Reiderjack, we think the best option here is to do neither a new damage type nor attack type. We will instead program a new behavior for the PKA which is very easy, I can help you with that in Discord.

corp-0 commented 3 years ago

adding DNM to avoid a speed merge. Some changes were requested.

yoysam commented 3 years ago

well lets hope its good this time

corp-0 commented 3 years ago

I just realized the class name has a typo, you wrote Kenetic rather than Kinetic.

corp-0 commented 3 years ago

image Kinetic bullet has no damage type assigned. Set it to brute, please.

yoysam commented 3 years ago

should work now

corp-0 commented 3 years ago

image Kinetic bullet has no damage type assigned. Set it to brute, please.

Following how TG did it, it looks like this applies bomb attack type. You can set that in this scriptable object too.

corp-0 commented 3 years ago

I have approved changes, if someone can please test it live it would be great. I'm heading bed now, if no one tested I will do it tomorrow morning.

If other reviewer tested and feels it is ok to merge, please do : )

Akbadain29 commented 3 years ago

I've tested it, the damage slightly increases as the pressure increases. I've reworked the formula On line 39 of KeneticProjectileDamageInterity.cs you've got newDamage = (1175 / 26) - ((15 - pressure) / 26); The gradient is too small and it won't have the desired effect. Instead, you should put: newDamage = 40 * (Mathf.Clamp((-pressure / 135), -1.0f, 0.0f) + 1); Mathf.Clamp will stop the float from going below 0 and causing negative damage if it exceeds approximately 140kPa. It will also deal 40 damage maximum in space if the pressure is 0kPa.

yoysam commented 3 years ago

math as been updated

yoysam commented 3 years ago

ok fixed those

ReiderJack commented 3 years ago

Keep local variables close to where they are used. You are getting pressure before null check which is very inefficient, because getting pressure value costs time and you don't know yet if you are gonna use it.

MoveItThere

yoysam commented 3 years ago

got it

corp-0 commented 3 years ago

Changes done, tests passed. I'm merging.