Realm #18

richard-pianka · 2023-04-04T02:51:58Z

No description provided.

…rd for logging onto expansion character with d2dv

richard-pianka · 2023-04-04T16:30:25Z

I'm not implementing games or ladder yet
It turns out MCP does send a protocol byte, except it's 0x01, so I stuck it on a different socket; I chose port 6113 because it's what pvpgn uses
Do you want the name "Olympus" as a setting?
Sending a statstring for EID_USERUPDATE causes a rather esoteric rendering bug in Diablo II
- We could make not sending it only a D2DV/D2XP thing if it's expected for other products
Take a look at SID_ENTERCHAT lines 73-90, see comment
The SID_LOGONREALMEX response has to send an IP address, I did a quick lookup but it's not exactly ideal...lmk if you're already getting this somewhere or you want a different solution (see NetworkUtilities.cs)
I'm using a bunch of static but essentially random values for the MCP chunks (only actually relying on the cookie), not sure what you want to do here
I had to essentially duplicate several classes: Message, Frame, one of the exceptions, the ServerSocket, etc.; any or all of these could be much more DRY with a refactor, lmk what you think (feels icky)
There are a handful of places I want to decompose into functions, add helpers, etc. - planning to hit that in my next pass of refactors if that's cool
I left a couple pieces of commented out code as reminders for places I need to treat when I add games and ladder
There are like 4-5 areas where I found the bnetdocs documentation could be improved, I can submit these later and trim out my comments

carlbennett

I'm not implementing games or ladder yet

That's fine.

It turns out MCP does send a protocol byte, except it's 0x01, so I stuck it on a different socket; I chose port 6113 because it's what pvpgn uses

That's fine.

Do you want the name "Olympus" as a setting?

Yes.

Sending a statstring for EID_USERUPDATE causes a rather esoteric rendering bug in Diablo II
We could make not sending it only a D2DV/D2XP thing if it's expected for other products

It's expected that only users on the same product as the initiator will receive other users' EID_USERUPDATE messages, I believe. Spht may know more specifically on that. I'd be fine with D2DV/D2XP having special behavior if it's warranted, but only for statstring, as flag updates still need to occur for when a user gains channel operator after /designate or becomes channel speaker, etc.

Take a look at SID_ENTERCHAT lines 73-90, see comment

Seems fine. Behavior is unchanged for now which I'm happy with. The comment seems like future work/todo.

The SID_LOGONREALMEX response has to send an IP address, I did a quick lookup but it's not exactly ideal...lmk if you're already getting this somewhere or you want a different solution (see NetworkUtilities.cs)

I've commented already but the URL that it tries needs to be made configurable and it needs a discriminator for the type of IP address it uses to learn its public IP address.

I'm using a bunch of static but essentially random values for the MCP chunks (only actually relying on the cookie), not sure what you want to do here

Not sure either. Seems like future work/todo. Static is fine at this stage where not everything is implemented.

I had to essentially duplicate several classes: Message, Frame, one of the exceptions, the ServerSocket, etc.; any or all of these could be much more DRY with a refactor, lmk what you think (feels icky)

I've left comments throughout those files. Please address them. DRY would be better, it is icky right now.

There are a handful of places I want to decompose into functions, add helpers, etc. - planning to hit that in my next pass of refactors if that's cool

I disagree with the premise of the ByteHelpers. I really want to look over each reason why you're adding one. The .NET standard library and C# language provide a lot already.

I left a couple pieces of commented out code as reminders for places I need to treat when I add games and ladder

Seems fine.

There are like 4-5 areas where I found the bnetdocs documentation could be improved, I can submit these later and trim out my comments

You asked to be made an editor on BNETDocs. I've already discussed this with you over Discord. I am not the sole decision maker on that. @nmbook and @Davnit also need to weigh their opinion as they are BNETDocs Staff, together all three of us make up a quorum. We've had editors make changes on the site without consultation and those changes ended up needing to be reversed, and I've made unilateral decisions in the past without asking others that ended up going bad for me. Fundamentally, I believe we'd rather moderate by having new changes made by the existing editors we have now. I'm open to opinions.

carlbennett · 2023-04-04T23:45:57Z

src/Atlasd/Battlenet/ClientState.cs

+            {
+                var partial = new byte[][]
+                {
+                    "Olympus".ToBytes(),


This should be made into a configurable. Perhaps you want to use this?

var realmName = Settings.GetString(new string[] { "realm", "name" });

carlbennett · 2023-04-05T00:18:55Z

src/Atlasd/Utilities/NetworkUtilities.cs

+    {
+        public static string GetPublicAddress()
+        {
+            WebRequest request = WebRequest.Create("https://api.ipify.org");


The hardcoded url should be made into a configurable option.

Around this line, there should also be a discriminator for IPv4, as the endpoint/url that a user may enter (or even override with their own DNS or hosts file) could contain an Dual-Stack (IPv4+IPv6 / A+AAAA) record.

Since Battle.net protocols only support IPv4, Atlas should make sure it only takes the IPv4 address, if both IPv4 and IPv6 are available from DNS, and it should error or give an empty/null/none result if there is only an IPv6 address with no IPv4 address from DNS or if DNS resolution fails.

carlbennett · 2023-04-05T00:30:40Z

src/Atlasd/Utilities/BytesHelper.cs

+
+namespace Atlasd.Helpers
+{
+    public static class BytesHelper


This class seems fundamentally not necessary, as C# and .NET 3.1 already provide for many of the bit and byte helpers with BitConverter to start.

The methods below, GetBytes, AsString, ToBytes, that extend UInt32[], IPAddress, byte[], and strings, already exist elsewhere via the .NET namespaces.

Further, AsString() and ToBytes() assumes a UTF8 encoding and does not contain a parameter to set the encoding, not all of Battle.net supported UTF8 and there are components of the protocol that in fact used Latin1 (ISO-8859-1) encoding, despite Atlas's incorrect emulation only using UTF8. Any improvement to the general helpfulness of converting strings to byte arrays and byte arrays to strings, should include an encoding parameter at minimum, though again this entire class seems not necessary.

carlbennett · 2023-04-05T00:31:22Z

src/Atlasd/Daemon/Logging.cs

@@ -85,6 +85,7 @@ public static void WriteLine(LogLevel level, LogType type, string buffer)
            if (level > CurrentLogLevel) return;

            Console.Out.WriteLine($"[{DateTime.Now}] [{LogLevelToString(level)}] [{LogTypeToString(type).Replace("_", "] [")}] {buffer}");
+            Console.Out.Flush();


I'm concerned about the performance of always calling Flush() when Debug log-level is set on a busy server.

carlbennett · 2023-04-05T00:32:41Z

src/Atlasd/Battlenet/RealmSocket.cs

+            SetLocalEndPoint(localEndPoint);
+        }
+
+        public void Dispose() /* part of IDisposable */


This needs to be refactored. It was copied from the original ServerSocket which is also wrong.

https://learn.microsoft.com/en-us/dotnet/api/system.idisposable?view=netcore-3.1

tl;dr - It needs to only Dispose during special conditions relating to the garbage collector, which this does not even consider.

carlbennett · 2023-04-05T00:55:02Z

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_LOGONREALMEX.cs

@@ -75,15 +77,84 @@ public override bool Invoke(MessageContext context)
                         * (STRING)     Battle.net unique name (* as of D2 1.14d, this is empty)
                         */

-                        Buffer = new byte[8]; // MCP Cookie + MCP Status only; Atlas does not have realm/MCP servers implemented yet.
+                        if (((byte[])context.Arguments["realmTitle"]).AsString() == "Olympus")


Make it configurable

carlbennett · 2023-04-05T00:55:33Z

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs

@@ -5,6 +5,7 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using Atlasd.Helpers;


Alphabetize the using list

carlbennett · 2023-04-05T00:56:48Z

src/Atlasd/Battlenet/Common.cs

+            Settings.State.RootElement.TryGetProperty("battlenet", out var battlenetJson);
+            battlenetJson.TryGetProperty("realm_listener", out var listenerJson);
+            listenerJson.TryGetProperty("interface", out var interfaceJson);
+            listenerJson.TryGetProperty("port", out var portJson);
+
+            var listenerAddressStr = interfaceJson.GetString();


This should be refactored using the settings getter.

string listenerAddressStr = Settings.Get(new string[] { "battlenet", "realm_listener", "interface", "port" }, default: string.Empty);

carlbennett · 2023-04-05T00:57:21Z

src/Atlasd/Battlenet/Common.cs

@@ -102,14 +107,17 @@ public static void Initialize()
            ActiveChannels = new ConcurrentDictionary<string, Channel>(StringComparer.OrdinalIgnoreCase);
            ActiveClans = new ConcurrentDictionary<byte[], Clan>();
            ActiveClientStates = new ConcurrentDictionary<Socket, ClientState>();
+            RealmClientStates = new ConcurrentDictionary<UInt32, ClientState>();


Alphabetize this

carlbennett · 2023-04-05T00:57:38Z

src/Atlasd/Battlenet/Common.cs

        public static List<GameAd> ActiveGameAds;
        public static ConcurrentDictionary<string, GameState> ActiveGameStates;
+        public static Realm Realm;


Alphabetize this by property name

richard-pianka added 12 commits April 2, 2023 20:36

feat(mcp): adding the basics of mcp and realm login

f56e2bd

chore(mcp): nixing the realm cookie from client state, superfluous

9b66228

chore(mcp): moving to RealmProtocolException for MCP packets

066acfd

chore(mcp): refactoring network byte order into helper method

aae0c86

fix(mcp): correcting placement of logging statement in logonrealmex

36a9b57

fix(mcp): setting realm name in statstring

6bfdd2c

chore(mcp): minor formatting tweak

e69b7b1

fix(etc): reverting a change to the default realm name

dd1a49c

feat(mcp): adding chardelete and charlogon packet support, adding gua…

69d4575

…rd for logging onto expansion character with d2dv

feat(mcp): adding legacy charlist packet support

20d8181

chore(etc): fixing indentation to match original

0c66edc

chore(etc): reverting incorrect realm name change

fde20f7

richard-pianka requested a review from carlbennett April 4, 2023 02:51

richard-pianka self-assigned this Apr 4, 2023

Merge branch 'develop' into realm

37577db

carlbennett requested changes Apr 5, 2023

View reviewed changes

carlbennett added enhancement New feature or request help wanted Extra attention is needed labels Apr 5, 2023

carlbennett self-assigned this Apr 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Realm #18

Realm #18

richard-pianka commented Apr 4, 2023

richard-pianka commented Apr 4, 2023

carlbennett left a comment •

edited

Loading

carlbennett Apr 4, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

carlbennett Apr 5, 2023

Realm #18

Are you sure you want to change the base?

Realm #18

Conversation

richard-pianka commented Apr 4, 2023

richard-pianka commented Apr 4, 2023

carlbennett left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlbennett left a comment •

edited

Loading