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

Potion of Oblivion vs FE Refund #8

Open
Hanter-19 opened this issue May 28, 2023 · 2 comments
Open

Potion of Oblivion vs FE Refund #8

Hanter-19 opened this issue May 28, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@Hanter-19
Copy link
Owner

Reported by Bella'kor on Discord: https://discord.com/channels/547043336465154049/1088956924516634707/1112341341452640316

To summarize the issue:

Current Behaviour

  • I have the FE Perk and I have achieved the required kills and refunded the Perk Point
  • I drink the Potion of Oblivion -> I lose the FE Perk; I gain 1 Perk Point
  • After drinking the potion, I choose the same FE Perk again -> Even though I already achieved the required kills, the FE Perk does not refund itself

Expected Behaviour

  • I have the FE Perk and I have achieved the required kills and refunded the Perk Point
  • I drink the Potion of Oblivion -> I lose the FE Perk; I gain 1 Perk Point
  • After drinking the potion, I choose the same FE Perk again -> Because I already achieved the required kills, the FE Perk refunds itself again

Code Analysis and Proposed Fix

Currently the FE Perk refund process checks for the existence of a flag in the actor, and will only refund the point if the flag is not already present. (see legend_favoured_enemy_skill.onUpdate())

When drinking Potion of Oblivion, the flags are not removed, so when the Player picks the FE Perk again, this check will detect the flags and not refund the Perk Point.

I propose we hook the entity/tactical/actor.resetPerks() function to also remove all FE Perk-related flags in the actor.

@Hanter-19 Hanter-19 added the bug Something isn't working label May 28, 2023
@Hanter-19 Hanter-19 self-assigned this May 28, 2023
@Hanter-19
Copy link
Owner Author

Hanter-19 commented May 28, 2023

First attempt to fix was tested by Bella'kor.

This time, we detected a bug where drinking the Potion of Oblivion resulted in the character getting refunded more points than they were supposed to based on the number of times they leveled up.

Suspected Cause

The resetPerks function iterates over all of the character's skills and counts any Skill object that is the Perk type. This means that additional Perks added after refunding FE perks will get included in this count, therefore inflating the number of perk points refunded by resetPerks

Proposed Solution

In the same hook that I wrote for resetPerks, add some logic to reduce the actor's total perk points (after resetPerks has been called). We will reduce the amount by the number of FE flags that we removed (therefore accounting for the inflated points)

Possible Edge Case?

My brain is too tired to work out the details right now, but we will need to consider what is the effect of the Proposed Solution for the following edge case:

  • Actor fulfils the FE Refund requirement and gains 1 Perk Point
  • Actor then drinks Potion of Oblivion before spending that 1 Perk Point

Will revisit this potential edge case next time and analyse the impact.

@Hanter-19 Hanter-19 pinned this issue May 28, 2023
@Hanter-19
Copy link
Owner Author

Edge Case Considered

By following the Proposed Solution outlined in the previous comment, the Possible Edge Case raised would be handled.

Consider the following example:

  • A character has spent 2 Perk Points, 1 on a Favoured Enemy Perk and 1 on some other refundable perk:
Number of Perks Owned Number of Available Perk Points
2 0
  • The character has just fulfilled the requirement to refund the Favoured Enemy Perk Point:
Number of Perks Owned Number of Available Perk Points
2 1
  • The character drinks the Potion of Oblivion and resets their perk points, which initially results in the following:
Number of Perks Owned Number of Available Perk Points
0 3
  • Following the Proposed Solution, we would have counted 1 Flag for having fulfilled the Favoured Enemy Refund, so we would subtract 1 Perk Point from the Number of Available Perk Points, resulting in the following:
Number of Perks Owned Number of Available Perk Points
0 2

Thus, we have successfully started with 2 Perks Owned and ended with 2 Available Perk Points.

The above illustration shows that our Edge Case is handled by the Proposed Solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant