Skip to content

Don't save default minion skill set choice #7099

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

Closed

Conversation

Paliak
Copy link
Contributor

@Paliak Paliak commented Dec 18, 2023

Description of the problem being solved:

Default minion skillset was being saved to skills triggering phantasm support causing the default minion skill set to spread like a disease in build save files.

Marking as draft as while rather innocent still sort of worried it may break some edge case.

@LocalIdentity LocalIdentity changed the title Don't save defualt minion skill set choice Don't save default minion skill set choice Dec 18, 2023
@QuickStick123
Copy link
Contributor

#5700
#5571
#5697

Here are some related PRs.

@QuickStick123
Copy link
Contributor

QuickStick123 commented Dec 19, 2023

AFAIK this should only be occurring with disabled skills/vaal skills atm. #5700 has details on the issues I still found with it.

@Paliak
Copy link
Contributor Author

Paliak commented Dec 19, 2023

@QuickStick123 This happens with all skills. This is caused by the drop down sorting functionality adding the summon phantasms support which saves the default selection of minion skill in the drop down to the skillInstance which is then later saved to the build xml.

@Paliak Paliak added the technical Hidden from release notes label Dec 19, 2023
@QuickStick123
Copy link
Contributor

QuickStick123 commented Dec 19, 2023

Do you have some replication steps? I was under the impression I fixed this for everything except vaal skills which is fixed in #5700.

The only othercases are with disabled skills or oddities on support gems.

The disabled skills could probably fixed by calling the build functions on all skills before saving.

@Paliak
Copy link
Contributor Author

Paliak commented Jul 17, 2024

This now causes the minion selection to not be saved to build xml which is probably not worth the minor build xml bloating. Closing in favor of #5700 (and to clean up the opened prs a bit)

@Paliak Paliak closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical Hidden from release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants