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

Fix wrong Item Type for Titan Camos #728

Merged
merged 10 commits into from
Nov 3, 2023

Conversation

Zanieon
Copy link
Contributor

@Zanieon Zanieon commented Sep 29, 2023

Split from #726

Small fix which the wrong item type was being addressed to check for Titan Camos, allowing players to use any camo without fallback to default if they didn't own that item previously when enabling progression.

@ASpoonPlaysGames
Copy link
Contributor

Why must respawn use different item types for copies of the same item i may never know

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, can't test though for the next few days so yeah

@ASpoonPlaysGames ASpoonPlaysGames added the needs testing Changes from the PR still need to be tested label Sep 29, 2023
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.

Can't really reproduce the bug in testing. @Zanieon suggested to disable progression, select a Titan camo that would not yet be unlocked, enabled progression again and the Titan would keep the locked camo.

That however didn't really happen for me. Both with and without the PR, the Titan would always switch back to a the default camo if the previously set camo was a locked one.

I'm gonna leave this to you @ASpoonPlaysGames ^^

@ASpoonPlaysGames
Copy link
Contributor

Works in testing, couldn't reproduce the issue, likely because all camos are registered with pilot, weapon, and titan items.
image

So in my testing this PR didn't change anything, but the code change is good so uhhh, lmao

@ASpoonPlaysGames ASpoonPlaysGames added READY TO MERGE This mergeable right now and removed needs testing Changes from the PR still need to be tested labels Oct 20, 2023
@GeckoEidechse
Copy link
Member

Merging based on @ASpoonPlaysGames' review ig

@GeckoEidechse GeckoEidechse merged commit dfeaba7 into R2Northstar:main Nov 3, 2023
3 checks passed
@Zanieon Zanieon deleted the progression_index_fix branch November 3, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants