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

add -p/--slippi-port cmd line option for spectate server port #421

Merged
merged 5 commits into from
Jun 6, 2024

Conversation

jmlee337
Copy link
Contributor

@jmlee337 jmlee337 commented Jun 2, 2024

fixes #419

this will allow developers to connect to the spectate server on multiple dolphin instances on the same machine

Source/Core/DolphinWX/Main.cpp Outdated Show resolved Hide resolved
Copy link
Member

@NikhilNarayana NikhilNarayana left a comment

Choose a reason for hiding this comment

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

apologies for suddenly changing my mind, i re-read it and decided i'd rather have this be very explicit about what port it refers to

Source/Core/DolphinWX/Main.cpp Outdated Show resolved Hide resolved
Source/Core/DolphinWX/Main.h Outdated Show resolved Hide resolved
Source/Core/DolphinWX/Main.h Outdated Show resolved Hide resolved
Source/Core/DolphinWX/Main.cpp Outdated Show resolved Hide resolved
Source/Core/DolphinWX/Main.cpp Outdated Show resolved Hide resolved
@jmlee337
Copy link
Contributor Author

jmlee337 commented Jun 4, 2024

looks like there's a precedent for 'slippi spectate' as in SlippiSpectateServer:

class SlippiSpectateServer
is this ok?

@NikhilNarayana
Copy link
Member

NikhilNarayana commented Jun 4, 2024

looks like there's a precedent for 'slippi spectate' as in SlippiSpectateServer:

class SlippiSpectateServer

is this ok?

not necessarily against it, but i would like the corresponding config value to be renamed to match so it is easy to match them up, if you prefer just spectate

@jmlee337
Copy link
Contributor Author

jmlee337 commented Jun 5, 2024

oh the config name lol oops. No I'll go spectator I don't want to change the config name that could be breaking if people are touching it programmatically

@NikhilNarayana NikhilNarayana merged commit ff2d4d1 into project-slippi:slippi Jun 6, 2024
8 checks passed
@jmlee337 jmlee337 deleted the port branch June 6, 2024 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When using multiple playback dolphins, every SlippiSpectateServer after the first fails to start
2 participants