-
Notifications
You must be signed in to change notification settings - Fork 114
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 hit chance calculation and armor rating display #601
base: master
Are you sure you want to change the base?
Conversation
src/brogue/PowerTables.c
Outdated
short idx = clamp(netDefense * 4 / 10 / FP_FACTOR + 80, 0, LAST_INDEX(POW_DEFENSE_FRACTION)); | ||
// Adding "FP_FACTOR * 5 / 4" here is necessary to ensure that an armor rating of 3.75 is different from 3.5. It doesn't change the result for armor ratings that are multiples of 0.5, but increases the index by 1 for armor ratings that end in 0.25 or 0.75. | ||
short idx = clamp((netDefense + FP_FACTOR * 5 / 4) * 4 / 10 / FP_FACTOR + 80, 0, LAST_INDEX(POW_DEFENSE_FRACTION)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is giving me slightly bad vibes. Gone from adding 0 to 5/4. Busy now but I need to have another look later.
(If in debugging this issue you wished you could redo the entire armor system, I support you in this, seems some strange code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a long comment explaining what this code does (or trying to, anyway). I also modified this line to replace the ugly 5/4 constant with 0.5, while keeping the same behavior. It's still not perfect, but hopefully it's understandable now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a gameplay issue here? If so, how would one duplicate it in wizard mode? I'm not keen on the UI change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a gameplay issue here? If so, how would one duplicate it in wizard mode? I'm not keen on the UI change.
The issue is that armor increments of 0.25 don't affect enemy hit chance (but increments of 0.5 do). To repro, create an enemy goblin (or similar) and several potions of strength, and check the monster's hit chance in between drinking strength potions. Only every other strength potion has an effect on its hit chance. This is inconsistent with your own hit chance, which increases slightly with each strength potion.
What don't you like about the UI change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok that makes sense. On the UI changes, it's the change to the armor value in the info panel (top left) that I'm resistant to. There's a satisfying simplicity to the integer value.
I'm not opposed to changing the item details screen though, both for the equipped armor and when comparing an armor in inventory to the equipped armor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I like the idea of displaying the armor value with different precision in different places. I see your point about integers being simpler, but I think we should look for ways to keep the UI simple without putting the non-rounded armor value on a separate screen.
What about multiplying all displayed armor values by 4? The starting armor value would be 3.5*4 = 14, plate armor at 19 strength would have an armor value of 44, etc. Players would need to re-learn what armor values mean, though; I don't know whether it's worth it.
Fixes the hit chance calculation for armor ratings ending in 0.25 and 0.75, and avoids rounding armor ratings to integers in the UI. It displays 1 decimal place, so 3.75 is displayed as 3.7, but this is probably fine since it doesn't make much difference. Fixes tmewett#596
2c88104
to
377e222
Compare
Fixes the hit chance calculation for armor ratings ending in 0.25 and 0.75, and avoids rounding armor ratings to integers in the UI. It displays 1 decimal place, so 3.75 is displayed as 3.7, but this is probably fine since it doesn't make much difference.
Fixes #596