Path-of-Terraria / PathOfTerraria

GNU General Public License v3.0
1 stars 3 forks source link

Showing -1 for Point Remaining @ Level 0 #389

Closed CollinHerber closed 1 month ago

CollinHerber commented 1 month ago

Description

When level 0 it should show "0" Points remaining, not -1

image

Reproduction steps

No response

Screenshots

No response

Logs

No response

CollinHerber commented 1 month ago

Looks like PassiveTreePlayer.Points is -1 but I'm not sure what is setting it since the default should be 0.

ckeyboard commented 1 month ago

Is there a reliable way to reproduce this bug?

ckeyboard commented 1 month ago

Steps to Reproduce

  1. Create a 'TestDummy' character
  2. Open any saved world
  3. Open the inventory menu
  4. Click on the skill tree bar at the top center of the screen
    • Observe that the skill point number is 0
  5. Click on Settings
  6. Inside the Settings Menu, click on Save & Quit
  7. Open any saved world
  8. Open the inventory menu
  9. Click on the skill tree bar at the top center of the screen
    • Observe that the skill point number is now -1

Cause

The issue is caused by AnchorPassive being set to level 1 in PassiveElement.cs:

// Anchor passive should always be "unlocked", thus this hardcoding
if (_passive is AnchorPassive)
{
    _passive.Level = 1;
}

This seems to be a hackish way of guaranteeing AnchorPassive exists.

Solution

The easy solution to this is to either:

  1. remove this code
  2. or explicitly set this value to zero

If we go with option 2, characters still affected by the contaminated value can be fixed with the following steps:

  1. open any world
  2. save and quit to sanitize the character save file

However, not having the AnchorPassive level 1 guarantee anymore could cause side-effects.

CollinHerber commented 1 month ago

Steps to Reproduce

  1. Create a 'TestDummy' character
  2. Open any saved world
  3. Open the inventory menu
  4. Click on the skill tree bar at the top center of the screen

    • Observe that the skill point number is 0
  5. Click on Settings
  6. Inside the Settings Menu, click on Save & Quit
  7. Open any saved world
  8. Open the inventory menu
  9. Click on the skill tree bar at the top center of the screen

    • Observe that the skill point number is now -1

Cause

The issue is caused by AnchorPassive being set to level 1 in PassiveElement.cs:

// Anchor passive should always be "unlocked", thus this hardcoding
if (_passive is AnchorPassive)
{
    _passive.Level = 1;
}

This seems to be a hackish way of guaranteeing AnchorPassive exists.

Solution

The easy solution to this is to either:

  1. remove this code
  2. or explicitly set this value to zero

If we go with option 2, characters still affected by the contaminated value can be fixed with the following steps:

  1. open any world
  2. save and quit to sanitize the character save file

However, not having the AnchorPassive level 1 guarantee anymore could cause side-effects.

Thank you for the research into this. Shouldn't this check prevent it from going into the -1? https://github.com/Path-of-Terraria/PathOfTerraria/blob/main/Common/Systems/PassiveTreeSystem/PassiveTreeSystem.cs#L89

Additionally I'm down to update the anchor code to not be an allocatable passive but rather just something you get for free as a starting area. However we want to accomplish that. Might make that easier to handle

ckeyboard commented 1 month ago

The reason that code doesn't catch it is because AnchorPassive is not blacklisted like Anchor.

if (passive.InternalIdentifier != "Anchor" && passive.Level > 0)

That check would need to include something like:

if (passive.InternalIdentifier != "AnchorPassive" && passive.InternalIdentifier != "Anchor" && passive.Level > 0)

What I would like to know is what does AnchorPassive and Anchor actually do?

GabeHasWon commented 1 month ago

They are the "origin" node of the respective skill trees. Anchor is just the skill tree anchor, and AnchorPassive is for SkillPassives, which are almost the same system. Neither do anything but hold connections to other nodes.

CollinHerber commented 1 month ago

Essentially what Gabe said. They are the starting point of the tree. This is also what is used to see if a passive is able to be allocated or not, it tries to reach the anchor through allocated nodes.

ckeyboard commented 1 month ago

Since there's logic that depends on these anchors, I think it best to leave them alone until it becomes clear that their design will cause significant problems.