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

Multiplicative Armor Stacking #13002

Merged
merged 6 commits into from
May 19, 2024
Merged

Multiplicative Armor Stacking #13002

merged 6 commits into from
May 19, 2024

Conversation

simb11
Copy link
Contributor

@simb11 simb11 commented Mar 27, 2024

Описание изменений

изменяет наложение брони из простой суммы на мультипликативное.
пример
сейчас:
имея комбез офицера (10%) и броню (50%) мы получали 60% защиты
теперь:
имея те же вещи мы получаем 55% защиты

Почему и что этот ПР улучшит

фикс бага
будет невозможно настакать (например с помощью комбеза офицера, риотки и пресса) 100% защиты

Авторство

буквально tgstation/tgstation#71396

Чеинжлог

🆑 Simbaka

  • tweak[link]: Изменена формула подсчёта защиты у брони.

@TauKitty
Copy link
Contributor

Changelog status: ✔️

@TauKitty TauKitty added the Fix label Mar 27, 2024
@simb11 simb11 added Tweak and removed Fix labels Mar 27, 2024
@LudwigVonChesterfield
Copy link
Contributor

ого

@simb11
Copy link
Contributor Author

simb11 commented Mar 27, 2024

поменял на дефайн

@Deahaka
Copy link
Contributor

Deahaka commented Mar 27, 2024

Жаль что нет курса на ультимативный иммунитет к урону при выполнении каких-то интересных условий...

if(C.body_parts_covered & BP.body_part)
protection *= PROTECTION_TO_MULTIPLE(C.armor[type])

return 100 - protection
Copy link
Contributor

Choose a reason for hiding this comment

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

Меня бесит что функция называется getarmor но возвращает она "процент урона который должен пройти"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

а как назвать тогда

Copy link
Contributor

Choose a reason for hiding this comment

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

я и не знаю даже, я уже так много на эти 5 строчек смотрел что всё поплыло

а оно точно сейчас корректно работает а не чем больше брони тем больше урона?

потому-что если смотреть как оно потом применяется в

https://github.com/TauCetiStation/TauCetiClassic/blob/master/code/modules/mob/living/carbon/human/human_defense.dm#L137
var/delta = max(0, P.damage - (P.damage * (armor/100)))

то выходит то что возвращает функция это на самом деле не armor, потому-что при armor = 70, дельта урона = 30%

но это и не protection_multiple, потому-что в той функции армор на неё не множится

ещё одна какая-то сущность и всё так дебильно!

мб назови функция get_protection_multiple_organ, сделай чтобы она возвращала в пределах от 0.0 до 1.0, функции которые её используют тоже переименуй, и в human_defense сделай var/delta = max(0, P.damage - P.damage * get_protection_multiple())

так вроде (лично мне) всё было бы максимально понятно.

но может просто кому-то ещё надо на это всё посмотреть

Copy link
Contributor Author

Choose a reason for hiding this comment

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

так?

Copy link

Данный ПР автоматически отмечен как застоявшийся по причине длительного отсутствия обновлений. Он будет закрыт через 7 дней, если никакой активности не будет проявлено. Если вы считаете, что ПР еще актуален, или что я (злобный робот) пристаю к вам зря - просто напишите любой комментарий. Спасибо за ваш вклад.

@LudwigVonChesterfield
Copy link
Contributor

нормальный ПР но надо чтобы кто-то другой посмотрел своим глазом

Copy link

Данный ПР автоматически отмечен как застоявшийся по причине длительного отсутствия обновлений. Он будет закрыт через 7 дней, если никакой активности не будет проявлено. Если вы считаете, что ПР еще актуален, или что я (злобный робот) пристаю к вам зря - просто напишите любой комментарий. Спасибо за ваш вклад.

Copy link

github-actions bot commented May 3, 2024

ПР закрыт из-за длительного отсуствия активности. Для переоткрытия ПРа, пожалуйста, обратитесь к кому-либо из мейнтейнеров. Вы можете призвать их в комментарии слапнув @TauCetiStation/maintainers.

@github-actions github-actions bot closed this May 3, 2024
@simb11 simb11 reopened this May 7, 2024
@simb11 simb11 removed the Stalled PR label May 7, 2024
@simb11
Copy link
Contributor Author

simb11 commented May 7, 2024

@Chip11-n переделал, теперь всё работает.

@simb11 simb11 requested a review from Chip11-n May 13, 2024 13:28
Copy link
Contributor

@Chip11-n Chip11-n left a comment

Choose a reason for hiding this comment

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

Меня все ещё прикалует 100 - 100 * (100 - min(p, 100) * 0.01) для расчёта брони. Честно, даже не знаю что с этой страшилой делать.
По самому балансу не вижу проблем.

Думаю можно в ТМ.

@LudwigVonChesterfield
Copy link
Contributor

image

Формула из Доты сделайте чтобы считалось как в Доте

code/modules/mob/living/carbon/human/human_defense.dm Outdated Show resolved Hide resolved
@@ -216,18 +217,16 @@
return siemens_coefficient

//this proc returns the armour value for a particular external organ.
/mob/living/carbon/human/proc/getarmor_organ(obj/item/organ/external/BP, type)
/mob/living/carbon/human/proc/get_protection_multiple_organ(obj/item/organ/external/BP, type)
Copy link
Member

Choose a reason for hiding this comment

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

имхо после переименования и fbbfbc6 стало запутанней.

Особенным был getHalLoss, а не getarmor, а теперь у нас дважды конвертируется значение из "+" в "-" и обратно: в getarmor, а потом через

//helper for inverting armor blocked values into a multiplier
#define blocked_mult(blocked) max(1 - (blocked / 100), 0)

Впрочем, виной всему отсутствующий долгое время рефакторинг в этой части кода, что оно плохо читаемо и тут сложно въехать и случайно что-то не сломать.

Сейчас по мне выглядит ок (не считая комментария выше), и можно дальше лишний раз не трогать.

Co-authored-by: Alexander V. <volas@ya.ru>
@simb11 simb11 requested a review from volas May 18, 2024 15:24
@volas volas merged commit 7616958 into TauCetiStation:master May 19, 2024
12 checks passed
TauKitty added a commit to TauCetiStation/ClassicChangelog that referenced this pull request May 19, 2024
@simb11 simb11 deleted the armor-multp branch June 6, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants