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.

Sign up for GitHub

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

Jump to bottom

Fix rpak aliasing not working when trying to alias towards a vanilla rpak #825

Merged
merged 2 commits into from
Oct 6, 2024
Merged

Fix rpak aliasing not working when trying to alias towards a vanilla rpak #825

merged 2 commits into from
Oct 6, 2024

Conversation

ASpoonPlaysGames
Copy link
Contributor

Fixing stuff described by @Alystrasz on discord. I don't really like this behaviour as it behaves a bit differently for vanilla rpak loads and modded rpak loads, but aliasing is hacky already so meh.

@github-actions github-actions bot added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Oct 5, 2024
@GeckoEidechse
Copy link
Member

cc @Alystrasz for testing

@Alystrasz
Copy link
Contributor

Explanation from Spoon on Discord:

it only worked before because of two bugs in the old system

which meant that when you aliased an rpak, the game would look in the vanilla folder for it first instead of the modded one, but then when it tried to load the vanilla one, it loaded the modded one as well

new system only looks in the modded folder for the alias, so for s2s where it is trying to alias a non-existent rpak to a vanilla rpak that behaviour is broken

Alystrasz
Alystrasz approved these changes Oct 5, 2024
View reviewed changes
Copy link
Contributor

@Alystrasz Alystrasz left a comment

Choose a reason for hiding this comment

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

To test this change, I installed s2space, a mod aliasing vanilla rpaks; then launched a game in private match using map mp_s2s.

Without PR

image

With PR

image

@GeckoEidechse GeckoEidechse removed the needs testing Changes from the PR still need to be tested label Oct 5, 2024
uniboi
uniboi approved these changes Oct 6, 2024
View reviewed changes
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.

Code looks good. Maybe a comment that mentions that vanilla aliases behave a bit different?

@GeckoEidechse GeckoEidechse removed the needs code review Changes from PR still need to be reviewed in code label Oct 6, 2024
@GeckoEidechse
Copy link
Member

cc @ASpoonPlaysGames to decide whether to merge with or without comment

@ASpoonPlaysGames
Copy link
Contributor Author

Dont think it's necessary, it acts fairly intuitively

@GeckoEidechse GeckoEidechse merged commit 42d9702 into R2Northstar:main Oct 6, 2024
6 checks passed
@GeckoEidechse
Copy link
Member

ah fuck, forgot to prefix commit with core:. Sorry @F1F7Y :c

wolf109909 pushed a commit to R2NorthstarCN/NorthstarLauncher that referenced this pull request Nov 12, 2024
@ASpoonPlaysGames @wolf109909
Fix rpak aliasing not working when trying to alias towards a vanilla …
05fadae 
…rpak (R2Northstar#825)

Replace incorrect variable with hardcoded `-1`
Don't account for the modded path in pak aliases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uniboi uniboi uniboi approved these changes

@Alystrasz Alystrasz Alystrasz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Northstar Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ASpoonPlaysGames @GeckoEidechse @Alystrasz @uniboi