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

Remove "Launch Multiplayer" button #719

Merged
merged 4 commits into from
Nov 5, 2023

Conversation

ASpoonPlaysGames
Copy link
Contributor

closes #711

Small localisation change, hopefully will reduce some tickets?

Will also be nice for if/when we get vanilla compatibility

@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 Sep 23, 2023
@GeckoEidechse GeckoEidechse added the feedback wanted Feedback is wanted whether the changes by this PR are desired label Oct 7, 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.

I want some feedback from other contributors on that. I agree that having Launch Multiplayer in there is kinda stupid.

On the flipside having Launch Vanilla in there and then locking it makes it seem that we are "trying to prevent players from playing vanilla" or are "planning to add vanilla support" or sth (which tbf is a whole-ass different discussion that currently goes into the direction of "maybe" I think?

Maybe we should just remove the non-Northstar button entirely?

TL;DR: better than before I guess. We could do better still IMO. Whether that's this PR or a different one, I guess that's the question ^^

@Jan200101
Copy link

In my opinion, either remove the Vanilla Multiplayer button entirely or rename it to vanilla.

@KyleGospo
Copy link
Contributor

Remove entirely

@H0L0theBard
Copy link
Contributor

I'd remove entirely unless we get around to adding proper vanilla support in the near future (which is not in 2032 when Northstar 2.0 releases).

@Erlite
Copy link
Contributor

Erlite commented Oct 14, 2023

Remove entirely, or rename to "Launch Official Multiplayer". Or if possible, could we just add a popup when you click it that tells you you're heading to official servers and not community ones?

@itscynxx
Copy link
Contributor

itscynxx commented Oct 14, 2023

i like the idea of removing it more, creates less confusion than what we have now while also not trying to make it look like vanilla compat is "just around the corner" or something

@Zanieon
Copy link
Contributor

Zanieon commented Oct 15, 2023

I vote for temporary removal, until we get vanilla compatibility working.

@ASpoonPlaysGames
Copy link
Contributor Author

Yeah, gonna rewrite this to remove the button given the feedback

@ASpoonPlaysGames ASpoonPlaysGames removed the feedback wanted Feedback is wanted whether the changes by this PR are desired label Oct 15, 2023
@ASpoonPlaysGames ASpoonPlaysGames changed the title change Launch Multiplayer to Launch Vanilla Remove "Launch Multiplayer" button Oct 15, 2023
Copy link
Contributor

@Zanieon Zanieon left a comment

Choose a reason for hiding this comment

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

Working without problems:
image

@Jan200101 Jan200101 removed the needs testing Changes from the PR still need to be tested label Oct 15, 2023
Copy link

@Jan200101 Jan200101 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, just comments out the multiplayer button and all relevant code with comments that explain why.

@Jan200101 Jan200101 added READY TO MERGE This mergeable right now and removed needs code review Changes from PR still need to be reviewed in code labels Oct 15, 2023
@GeckoEidechse
Copy link
Member

Merging based on reviews and discussion.

@GeckoEidechse GeckoEidechse merged commit 5076bea into R2Northstar:main Nov 5, 2023
3 checks passed
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.

"Launch Multiplayer" button isn't descriptive enough
8 participants