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 INVALID_REF errors caused by mods #657

Merged
merged 14 commits into from
Oct 12, 2023

Conversation

ASpoonPlaysGames
Copy link
Contributor

INVALID_REF errors were being caused by trying to get the item image for weapon skins.

If a mod (moreskins) added new skins, the client equipped one, and then the mod is disabled, the client now has bad persistence.
This is mostly handled fine by the game, except for weapon skins. This PR prevents the crash, and resets the bad persistence to default.

Testing:

  • Install MoreSkins and equip a weapon warpaint that does not exist in vanilla (example being masterwork for the r201)
  • Make sure your persistence has saved (close the game and reopen it, validate that the skin is still there)
  • Delete/disable MoreSkins
  • Go to the loadout, the weapon should now have Factory Issue equipped

Note: This PR introduces two vanilla files, menu_pilot_loadouts_shared.nut and menu_edit_pilot_loadouts.nut to Northstar.Client/mod/scripts/vscripts/ui, they will need to be committed as Respawn.

@F1F7Y
Copy link
Member

F1F7Y commented Jun 27, 2023

Followed instructions and the skin defaults to factory issue even when MoreSkins is enabled:
image

After disabling the mod:
image

@ASpoonPlaysGames
Copy link
Contributor Author

tf... ill look into that tomorrow then

@ASpoonPlaysGames
Copy link
Contributor Author

Followed instructions and the skin defaults to factory issue even when MoreSkins is enabled:

That looks normal? I probably should've mentioned that the actual gun skin looks like default, it's the icon thing you should be looking at

@ASpoonPlaysGames
Copy link
Contributor Author

After disabling the mod:
image

I can't replicate this? Did you have any other mods installed when testing this that might've been causing issues?

@F1F7Y
Copy link
Member

F1F7Y commented Jun 28, 2023

I can't replicate this? Did you have any other mods installed when testing this that might've been causing issues?

Flightcore overwrote the folder so i cant check, Ill test it again tho

@ASpoonPlaysGames
Copy link
Contributor Author

Flightcore overwrote the folder so i cant check, Ill test it again tho

I would normally just add a check to prevent this crash, but this code needs to run, or you'll just get a crash later on. I might throw an error that's a bit more descriptive though

@F1F7Y
Copy link
Member

F1F7Y commented Jun 28, 2023

I can't replicate this? Did you have any other mods installed when testing this that might've been causing issues?

Having the masterwork skin selected and going into the "Pilot loadouts" menu with MoreSkins disabled causes this

@ASpoonPlaysGames
Copy link
Contributor Author

ASpoonPlaysGames commented Jun 28, 2023

Having the masterwork skin selected and going into the "Pilot loadouts" menu with MoreSkins disabled causes this

Still can't replicate on my end unfortunately

EDIT: ok I can replicate now

Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

Couldn't manage to break it, changes look good

NOTE: Adds vanilla files

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.

@ASpoonPlaysGames from which VPK are the files from? ^^

@ASpoonPlaysGames
Copy link
Contributor Author

@ASpoonPlaysGames from which VPK are the files from? ^^

englishclient_frontend

@F1F7Y F1F7Y added the almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge label Jun 29, 2023
@F1F7Y
Copy link
Member

F1F7Y commented Jul 9, 2023

@GeckoEidechse bump

Copy link
Contributor

@uniboi uniboi left a comment

Choose a reason for hiding this comment

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

please please please the code is good and does what it's supposed to

@ASpoonPlaysGames ASpoonPlaysGames added READY TO MERGE This mergeable right now commits vanilla file For PRs that commit vanilla files from VPKs and removed almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Sep 2, 2023
@GeckoEidechse GeckoEidechse removed the commits vanilla file For PRs that commit vanilla files from VPKs label Oct 12, 2023
@GeckoEidechse
Copy link
Member

Merging based on past reviews.

@GeckoEidechse GeckoEidechse merged commit 407c5ce into R2Northstar:main Oct 12, 2023
3 checks passed
wolf109909 pushed a commit to R2NorthstarCN/NorthstarMods that referenced this pull request Nov 13, 2023
`INVALID_REF` errors were being caused by trying to get the item image for weapon skins.

If a mod (e.g. moreskins) added new skins, the client equipped one, and then the mod is disabled, the client now has bad persistence.
This is mostly handled fine by the game, except for weapon skins. This PR prevents the crash, and resets the bad persistence to default.
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.

4 participants