fixed multiple realms selection #130
Conversation
| @@ -30,7 +30,7 @@ class Config { | |||
| this.timezone = 0; | |||
|
|
|||
| this.locale = 'enUS'; | |||
| this.os = 'Mac'; | |||
| this.os = 'Win'; | |||
There was a problem hiding this comment.
Can you point to where this Warden check is implemented in either TrinityCore or cmangos?
There was a problem hiding this comment.
https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Server/WorldSession.cpp
In function
void WorldSession::InitWarden
However it's a check done in various part of code and trinitycore uses Win
and OSX as identifiers.
So Win is quite universal and should be used.
I've tested and it works
Sorry i'm from mobile now i can t link you the exact line
Il 23/Mar/2016 15:02, "fallenoak" notifications@github.com ha scritto:
In src/lib/config.js
#130 (comment):@@ -30,7 +30,7 @@ class Config {
this.timezone = 0;this.locale = 'enUS';
- this.os = 'Mac';
- this.os = 'Win';
Can you point to where this Warden check is implemented in either
TrinityCore or cmangos?—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/wowserhq/wowser/pull/130/files/f887684ea16fa977cddedffdb0ae858ed117cd6b..19910a55512dae698949a65042a549946fcd41f2#r57163394
There was a problem hiding this comment.
I see the following code, but I'm not sure I'm following why Warden fails if the identifier isn't one of 'Win' or 'OSX' -- to me, it looks like it just skips init'ing Warden altogether.
void WorldSession::InitWarden(BigNumber* k, std::string const& os)
{
if (os == "Win")
{
_warden = new WardenWin();
_warden->Init(this, k);
}
else if (os == "OSX")
{
// Disabled as it is causing the client to crash
// _warden = new WardenMac();
// _warden->Init(this, k);
}
}
There was a problem hiding this comment.
As i said you TC does multiple checks in core and if platform is not Win or OSX it doesn't allow you connecting to worldserver.
However if you don't believe me i've found the line for you: https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Server/WorldSocket.cpp#L471
There was a problem hiding this comment.
Right. Maybe this would be better as a separate commit, then? It doesn't seem related to fixing multiple realm handling, which is what the rest of your commit deals with.
There was a problem hiding this comment.
i've to do multiple PR or just separate the commit and include in same PR?
There was a problem hiding this comment.
I think separating the commit, but including it in the same PR would be great. I'm mostly thinking about what happens to tools like git blame when commits do too many separate things. :) @timkurvers What do you think?
|
Let's merge #120 first, as that contains the fix for allowing multiple realms to be handled correctly. It seems to be included in here, but it's difficult to see what's new. After that we can have a look at the Win vs OSX identifier, as well as Trinity requiring a realm ID to be passed in. |
|
i've splitted commit and changed that minor code-style |
|
could you accept this PR? it is fully working and made long time ago |
|
#120 was merged, which contained the original fix for supporting multiple realms. Could you rebase this on top of current master only including the TrinityCore fixes? |
|
Thanks 👍 |
and changed OS to pass warden check