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

Rework -vanilla to be a vanilla compatibility mode #601

Merged
merged 8 commits into from Dec 14, 2023

Conversation

ASpoonPlaysGames
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames commented Nov 18, 2023

Old -vanilla behaviour is now handled by -nonorthstardll

VANILLA is exposed as a squirrel constant, set to true when in vanilla compatibility mode.

Differences when in vanilla compatibility mode:

  • Doesn't restrict server commands (same as -norestrictservercommands)
  • Doesn't block FairFight screenshot functions
  • Doesn't do Atlas-related stuff (except for mainmenupromos)

With this PR various script changes will still be needed for proper vanilla compatibility, but it's a good start.

closes #588

@ASpoonPlaysGames ASpoonPlaysGames 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 Nov 18, 2023
@itscynxx
Copy link
Contributor

Are the auto compiled files not a good way to test this pr?

Using both -vanilla and -noworkerdll are both just launching regular vanilla on my end

@ASpoonPlaysGames
Copy link
Contributor Author

Are the auto compiled files not a good way to test this pr?

Using both -vanilla and -noworkerdll are both just launching regular vanilla on my end

It should work, make sure you grab both the exe and the dll since this PR touches both

@itscynxx
Copy link
Contributor

It should work, make sure you grab both the exe and the dll since this PR touches both

Even going from Northstar -> Delete exe and dll -> vanilla -> install exe and dll from builds, still not seeing anything with either launch option applied

Maybe its just a "womm" moment?

@ASpoonPlaysGames
Copy link
Contributor Author

ASpoonPlaysGames commented Nov 19, 2023

Using both -vanilla and -noworkerdll are both just launching regular vanilla on my end

Its weird that you are getting both doing the same thing, if -noworkerdll launches vanilla then even if -vanilla doesnt work how I want it to, the worst it could do is launch normal northstar...

How are you launching Northstar?

To clarify:

  • when trying to test -noworkerdll you should probably be launching through NorthstarLauncher.exe
  • when trying to test -vanilla you will still need -northstar as well if launching through steam/ea/whatever

@itscynxx
Copy link
Contributor

itscynxx commented Nov 19, 2023

To clarify:

  • when trying to test -noworkerdll you should probably be launching through NorthstarLauncher.exe

  • when trying to test -vanilla you will still need -northstar as well if launching through steam/ea/whatever

Wasn't aware -northstar was required as it wasn't required with -vanilla previously

-northstar -vanilla results in Northstar being launched with no main menu promos, with no Launch Multiplayer button (though this is handled in mods, so it makes sense). Trying to hit Launch Northstar results in a blank error message

It seems to be doing something, however I'm not sure to what degree it's meant to work without a mods pr

-northstar -nonorthstardll via Steam's launch options resulted in regular Northstar being launched with core mods and functional, just -nonorthstardll resulted in regular vanilla

-nonorthstardll passed to the .exe via a .bat file resulted in vanilla launching with the Northstar logging in a command prompt, which I'm pretty sure is the desired result. No mods, core or other, loaded. The entire log:
image

@ASpoonPlaysGames
Copy link
Contributor Author

-northstar -vanilla results in Northstar being launched with no main menu promos, with no Launch Multiplayer button (though this is handled in mods, so it makes sense). Trying to hit Launch Northstar results in a blank error message

It seems to be doing something, however I'm not sure to what degree it's meant to work without a mods pr

This is the intended result of this PR, yes

@GeckoEidechse
Copy link
Member

I'm a bit hesitant of changing the behaviour of an existing command as we have a lot of baggage of old tutorials that would need updating etc.

Would it make sense to use something like -vanillacompat for Northstar in vanilla compatible mode? And then after change -vanilla to -nonorthstardll and put a deprecation notice on -vanilla? Or is that too overkill?

@ASpoonPlaysGames
Copy link
Contributor Author

I'm a bit hesitant of changing the behaviour of an existing command as we have a lot of baggage of old tutorials that would need updating etc.

Literally nobody in the history of Northstar has used -vanilla intentionally for its old purpose. Definitely overkill

@GeckoEidechse
Copy link
Member

Aight, if you say this won't have a breaking impact, I'm fine with doing it in the current way ^^

@itscynxx
Copy link
Contributor

It should also be noted that this does not break VanillaPlus, in fact makes it possibly more straightforward for the end user installing it. Additionally, VanillaPlus can instead just use the VANILLA constant, not requiring having dependency checks for core mods, or doing individual checks for the core mods themselves

I have a 2.4 version of VanillaPlus using the VANILLA constant ready for when this gets released as well

@GeckoEidechse
Copy link
Member

Removing the testing label as @itscynxx already tested.

@GeckoEidechse GeckoEidechse removed the needs testing Changes from the PR still need to be tested label Dec 13, 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.

The code changes look fine from what I can tell, although I have to say that I only looked at what shows up in the diff, meaning I will have missed anything that was forgotten to be updated.

@GeckoEidechse GeckoEidechse added READY TO MERGE This mergeable right now and removed needs code review Changes from PR still need to be reviewed in code labels Dec 13, 2023
@GeckoEidechse GeckoEidechse merged commit 0976a35 into R2Northstar:main Dec 14, 2023
2 checks passed
@ASpoonPlaysGames ASpoonPlaysGames deleted the vanilla branch January 4, 2024 18:07
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.

Rework -vanilla launch option to be a vanilla compatibility mode
4 participants