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

Change Aegis Reset Functionality to use titan chassis ref instead of raw index #788

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

Zanieon
Copy link
Contributor

@Zanieon Zanieon commented Jan 23, 2024

Currently the index method reset does not correspond properly with the persistent data index, a method of picking titan by their class reference is the optimal idea so this ensures the correct titan will get their Aegis Levels to reset.

Related:

Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indices make sense in pdata, tbh it would be better for this ClientCommand to take a titanRef (from the titanClasses enum in pdef) instead of a raw index.

@Zanieon
Copy link
Contributor Author

Zanieon commented Jan 23, 2024

The indices make sense in pdata, tbh it would be better for this ClientCommand to take a titanRef (from the titanClasses enum in pdef) instead of a raw index.

Thanks for the suggestion, changed the method to this and its not swapping between the 3 loadouts due to UI loadout index mismatching with the pdata index.

Current method now is even better so that players can call the command from console and literally have to type the titan chassis name they want to reset (e.g: want to reset Monarch just use ns_resettitanaegis vanguard)

@Zanieon Zanieon changed the title Aegis Reset Functionality having wrong index loadouts Change Aegis Reset Functionality to use titan chassis ref instead of raw index Jan 23, 2024
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks better

@GeckoEidechse GeckoEidechse added the waiting on changes by author Waiting on PR author to implement the suggested changes label Jan 23, 2024
@Zanieon Zanieon removed the waiting on changes by author Waiting on PR author to implement the suggested changes label Jan 24, 2024
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nitpicks

Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gets Titan chassis id based on name which addresses the passed index not matching up with the one in pdef.

While function signature is updated in this PR, there's no code in Northstar yet that calls it, so no additional changes needed.

Skipping testing for this one cause it's cumbersome and the change appears to be obvious.

@GeckoEidechse GeckoEidechse merged commit 51ed776 into R2Northstar:main Jan 26, 2024
3 checks passed
@Zanieon Zanieon deleted the aegis_reset_fix branch January 26, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants