search

BGforgeNet / Fallout2_Unofficial_Patch

Fallout 2 Unofficial Patch, updated
https://forums.bgforge.net/viewforum.php?f=39
131 stars 9 forks source link

Fix is_critical checks #112

Closed burner1024 closed 4 months ago

burner1024 commented 1 year ago

is_critical is used in stat checks sometimes, that doesn't work. Need to check the code for this and replace with custom implementation.

burner1024 commented 1 year ago

It's actually worse: according to @NovaRain's research, is_critical in skill checks takes into account PC critical chance for calculating when to upgrade a success to a critical success. And critical chance is affected by perks/traits such as More Criticals. Which means combat perks affect non-combat mechanics, which I consider another bug. Maybe it should be fixed in sfall, or maybe the scripts can be switched to script-only critical implementation. Anyway, for now it's just a note to keep in mind.

NovaRain commented 1 year ago

is_critical in skill checks takes into account PC critical chance for calculating when to upgrade a success to a critical success.

It's roll_vs_skill script function takes PC critical chance into account when determining whether a success roll can be upgraded to a critical success. https://github.com/alexbatalov/fallout2-re/blob/main/src/int/support/intextra.c#L689 https://github.com/alexbatalov/fallout2-re/blob/main/src/game/skill.c#L424 https://github.com/alexbatalov/fallout2-re/blob/main/src/game/roll.c#L67 https://github.com/alexbatalov/fallout2-re/blob/main/src/game/roll.c#L83

burner1024 commented 1 year ago

Right, I misspoke. is_critical just checks the roll, mods are applied in actual roll.

burner1024 commented 5 months ago

I think I was wrong in my second comment, trait modifiers seem to be applied separately. And it does look like crit chance only counts on success roll. I guess... it's acceptable.

burner1024 commented 5 months ago

As far as I understand, ROLL enums are sequential integer values in engine, so it should be enough to replace just the one stat roll function.

burner1024 commented 4 months ago

I think I half-assed this. https://github.com/BGforgeNet/Fallout2_Unofficial_Patch/blob/4e3d2dc687345faba13e8af05c5267b25e31de47/scripts_src/headers/command.h#L492 should be is_success(do_check)

https://github.com/BGforgeNet/Fallout2_Unofficial_Patch/blob/4e3d2dc687345faba13e8af05c5267b25e31de47/scripts_src/template/spear_trap.ssl#L207-L211 Here perception roll still uses do_check. There are a few more cases like this.