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

Master server working again

Closed
wants to merge 1 commit into from
Closed

Master server working again #293

wants to merge 1 commit into from

Conversation

Stradex
Copy link
Contributor

@Stradex Stradex commented May 27, 2020

So this was the real issue after all. (the thing about si_version etc.. was nothing but a false-positive because I was testing after using the original Doom 3 steam version so probably the cache did saved my ip and port in the masterserver for a while)

@Stradex
Copy link
Contributor Author

Stradex commented May 27, 2020

A bit more of explanation why this works:

dhewm3 code did remove the following line from idAsyncServer::ProcessGetInfoMessage :

outMsg.WriteLong( fileSystem->GetOSMask() );

(Look at this: https://github.com/TTimo/doom3.gpl/blob/aaa855815ab484d5bd095f347163194ac569abcc/neo/framework/async/AsyncServer.cpp )

because GetOSMask does not exists anymore, but the master server expects that info to be received seems (even if there is no use for them maybe), so adding outMsg.WriteInt( -1 ); fixsthe problem.

@turol
Copy link
Contributor

turol commented May 27, 2020

You should add a comment about what this magic -1 is and why it is.

@Kalamatee
Copy link
Contributor

Kalamatee commented May 27, 2020

@DanielGibson
Copy link
Member

DanielGibson commented May 27, 2020

it would be nice if it was set using a cvar that could be overridden if necessary.

when should that be necessary?

To elaborate, this should only affect communication between multiplayer servers and the master server, which

  1. is hardcoded and can't be changed by the user
  2. just doesn't work right now without this change

The network protocol between multiplayer servers and clients playing on them should not be affected, so I don't think this breaks anything.

@Stradex
Copy link
Contributor Author

Stradex commented May 27, 2020

Can someone do the pull request with the comment for me?, I am too lazy for doing a pull request again to just adding a comment lol.

@DanielGibson
Copy link
Member

DanielGibson commented May 27, 2020

I can add the comment, of course :)

Note that you wouldn't have to create a new PR - you could just add a commit to the branch and push it (it gets added to the PR then) or even amend the existing commit and force-push the branch.

@Stradex
Copy link
Contributor Author

Stradex commented May 27, 2020

Yeah but I need to restart the pc (I am in linux now, I have everything in windows), open gitbash and aah, I am getting lazy to just think about it right now. haha

btw, the feature was already tested and it is working. Just to you know that this is not a feature without testing.

@DanielGibson
Copy link
Member

DanielGibson commented May 27, 2020

I pushed the commit to master (after adding a comment), thanks a lot! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants