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

Update session id logic #150

Merged
merged 11 commits into from
Mar 27, 2022

Conversation

sorokya
Copy link
Contributor

@sorokya sorokya commented Mar 26, 2022

This branch updates the Session ID logic around Account, Character, Welcome, and Warp packets to closer match the original GameServer and reoserv.

There's a big issue around PacketBuilder.AddShort not letting you add a ushort though!

Copy link
Owner

@ethanmoffat ethanmoffat left a comment

Choose a reason for hiding this comment

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

What are the results of testing these changes against etheos/eoserv/GameServer? Normally I'd assume it works but I'd like you to confirm because of the changes to the InitReply enum and reading data around it.

You can use test.etheos.moffat.io:8078 for etheos, and moffat.io:8079 for gameserver

EOLib/Domain/Account/AccountActions.cs Show resolved Hide resolved
EOLib/Domain/Protocol/InitReply.cs Show resolved Hide resolved
EOLib/Net/PacketBuilder.cs Outdated Show resolved Hide resolved
@sorokya
Copy link
Contributor Author

sorokya commented Mar 26, 2022

What are the results of testing these changes against etheos/eoserv/GameServer? Normally I'd assume it works but I'd like you to confirm because of the changes to the InitReply enum and reading data around it.

You can use test.etheos.moffat.io:8078 for etheos, and moffat.io:8079 for gameserver

I’ll try it out with both :)

@sorokya
Copy link
Contributor Author

sorokya commented Mar 26, 2022

What are the results of testing these changes against etheos/eoserv/GameServer? Normally I'd assume it works but I'd like you to confirm because of the changes to the InitReply enum and reading data around it.
You can use test.etheos.moffat.io:8078 for etheos, and moffat.io:8079 for gameserver

I’ll try it out with both :)

Tested with eoserv and everything worked fine
I tried your test.etheos.moffat.io for etheos but the client couldn't connect.
GameServer is bombing on account create. I'll try to debug it further later

@sorokya sorokya requested a review from ethanmoffat March 26, 2022 20:58
@ethanmoffat
Copy link
Owner

What are the results of testing these changes against etheos/eoserv/GameServer? Normally I'd assume it works but I'd like you to confirm because of the changes to the InitReply enum and reading data around it.
You can use test.etheos.moffat.io:8078 for etheos, and moffat.io:8079 for gameserver

I’ll try it out with both :)

Tested with eoserv and everything worked fine I tried your test.etheos.moffat.io for etheos but the client couldn't connect. GameServer is bombing on account create. I'll try to debug it further later

Try etheos-test.westus3.azurecontainer.io:8078...apparently my pipeline isn't deleting old dns entries so I've got a bunch of stale IP addresses on the test.etheos.moffat.io A record 😬

@@ -127,47 +129,47 @@ public async Task LoginToCharacter(ICharacter character)
if (unableToLoadMap || _fileRequestActions.NeedsFileForLogin(InitFileType.Map, _currentMapStateProvider.CurrentMapID))
{
gameLoadingDialog.SetState(GameLoadingDialogState.Map);
if (!await SafeGetFile(async () => await _fileRequestActions.GetMapFromServer(_currentMapStateProvider.CurrentMapID)))
if (!await SafeGetFile(async () => await _fileRequestActions.GetMapFromServer(_currentMapStateProvider.CurrentMapID, sessionID)))
Copy link
Owner

Choose a reason for hiding this comment

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

For these lines, do this instead of creating an async lambda:

if (!await SafeGetFile(() => _fileRequestActions.GetMapFromServer(_currentMapStateProvider.CurrentMapID, sessionID)))
    return;

It reduces the overhead in the async state machine when you return the task directly, since it is eventually await anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@sorokya
Copy link
Contributor Author

sorokya commented Mar 27, 2022

What are the results of testing these changes against etheos/eoserv/GameServer? Normally I'd assume it works but I'd like you to confirm because of the changes to the InitReply enum and reading data around it.
You can use test.etheos.moffat.io:8078 for etheos, and moffat.io:8079 for gameserver

I’ll try it out with both :)

Tested with eoserv and everything worked fine I tried your test.etheos.moffat.io for etheos but the client couldn't connect. GameServer is bombing on account create. I'll try to debug it further later

Try etheos-test.westus3.azurecontainer.io:8078...apparently my pipeline isn't deleting old dns entries so I've got a bunch of stale IP addresses on the test.etheos.moffat.io A record 😬

Re-tested with GameServer and etheos. Both work as intended :)

@sorokya sorokya requested a review from ethanmoffat March 27, 2022 10:04
@sorokya sorokya force-pushed the update-session-id-logic branch from 14a1aff to 0973919 Compare March 27, 2022 10:30
@sorokya sorokya force-pushed the update-session-id-logic branch from 0973919 to 08a9b2b Compare March 27, 2022 10:31
@ethanmoffat ethanmoffat merged commit f8a2ad0 into ethanmoffat:master Mar 27, 2022
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.

2 participants