Skip to content

Fix skillType tags for maximum Ballista Totem mods#5577

Merged
LocalIdentity merged 1 commit intoPathOfBuildingCommunity:devfrom
bobanobahoba:fix_max_ballista_totem_skilltype_tags
Apr 1, 2023
Merged

Fix skillType tags for maximum Ballista Totem mods#5577
LocalIdentity merged 1 commit intoPathOfBuildingCommunity:devfrom
bobanobahoba:fix_max_ballista_totem_skilltype_tags

Conversation

@bobanobahoba
Copy link
Contributor

@bobanobahoba bobanobahoba commented Jan 11, 2023

Fixes #5488 .

Description of the problem being solved:

Volcanic Fissure supported by Earthbreaker incorrectly gets +1 totem limit from "Attack Skills have +1 to maximum number of Summoned Ballista Totems" mods such as from Panopticon and Watchtowers.

Steps taken to verify a working solution:

  • Make a build with the skill gems Volcanic Fissure supported by Earthbreaker.
  • Allocate Panopticon and Watchtowers.
  • Check that the Active Totem Limit under the "Calcs" tab still shows 1.
  • As a sanity check, make a build with a bow and a bow attack skill gem supported by Ballista Support with the same passives allocated, and confirm that those still increase active totem limit.

Link to a build that showcases this PR:

https://pastebin.com/yzzh77Gp

Before screenshot:

image

After screenshot:

image

@QuickStick123
Copy link
Contributor

QuickStick123 commented Jan 11, 2023

This is also on cursed ground so I am not 100% sure on the naming of this SkillTag.
image
Maybe renaming it and requiring SummonsTotem might be a good idea to generalise this.

@QuickStick123 QuickStick123 added the bug: behaviour Behavioral differences label Jan 11, 2023
@bobanobahoba
Copy link
Contributor Author

I saw that as well, I was confused about cursed ground adding that tag because it doesn't really have anything to do with totem skills. At the moment all ballistas are bow attacks, and those nodes are processed that way by the " ["^attack skills [hd][ae][va][el] "] = { keywordFlags = KeywordFlag.Attack }, line in ModParser.lua .

@QuickStick123
Copy link
Contributor

The name is defined in Global.lua and skills.lua. If you don't have the exporter setup you could just replace all and it should work to update its meaning to something more consistent.
image
image

@bobanobahoba
Copy link
Contributor Author

bobanobahoba commented Jan 11, 2023

I'm not sure what tag the cursed ground tag was supposed to be (something to do with how there's maximum one, or that it starts persisting in an area like an orb?) so I'm not sure what I should change the cursed ground skilltype to. I think the TotemsAreBallistae is appropriate to keep for the ballista skills.

@LocalIdentity LocalIdentity changed the title Fix skillType tags for maximum ballista totem mods Fix skillType tags for maximum Ballista Totem mods Jan 30, 2023
@QuickStick123
Copy link
Contributor

Sorry got confused with how these are exported. Should be fine.

@LocalIdentity LocalIdentity merged commit b6db25d into PathOfBuildingCommunity:dev Apr 1, 2023
Dullson pushed a commit to Dullson/PathOfBuilding that referenced this pull request Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug: behaviour Behavioral differences

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panopticon gives +1 totem limit for skills supported by earthbreaker

3 participants