Closed Almighty-Shogun closed 2 years ago
It says a lot of "Moved ...
to ...
" but I want to tell I also cleaned code and made everything compatible with the new code-base.
I've tested the new code-base myself with the BasicExample
and FullClientExample
method from DiscordRPC.Example
and it worked. I haven't tested it in Unity since I have no acknowledgement of that. When the new code-base has been reviewed and approved I'll write the Migrate to v1.1
documentation for it and add more examples.
If any changes, improvements or something else to this code are needed, let me know and I'll do that asap.
Thank you for the time for splitting and sorting code out. I am not a fan of the chosen namespaces.. especially when it comes to the publicly facing classes. For example, DiscordRPC.RPC.Types.RPC.Assets
.... thats 3 instances of RPC and is 3 deep... which is too much in my opinion for commonly accessed APIs.
Additionally, everything in DiscordDPC.RPC
should be internal / raw RPC related functionality. Putting the Rich Presence object there is inappropriate in my opinion as it suggests its more complicated backend stuff than it actually is.
Here is my proposed changes:
DiscordRPC.RPC.Types.RPC
-> DiscordRPC.Entities
DiscordRPC.RPC.Types.Users
-> DiscordRPC.Entities
DiscordRPC.RPC.BaseRichPresence
-> DiscordRPC.Entities.BaseRichPresence
DiscordRPC.RPC.RichPresence
-> DiscordRPC.Entities.RichPresence
DiscordRPC
, but that doesn't make sense.DiscordRPC.Core
should be just DiscordRPC
.
Suggestions of other things to consider that i am unsure myself about:
DiscordRPC.RPC.RpcConnection
to DiscordRPC.RPC.Connection
State
.DiscordRPC.RPC.Messaging.Messages
-> DiscordRPC.RPC.Messaging
DiscordRPC.Core.Logging.Loggers
DiscordRPC.RPC.Payload.ServerEvent
-> DiscordRPC.RPC.ServerEvent
Did you make any code changes other than moving things around? If so let me know so I can review them too :)
Here is my proposed changes
I'll make an start on those changes today
Suggestions of other things to consider that i am unsure myself about:
RPC
in front it makes actually much more sense because it is like u said "already in the RPC namespace"Messaging.Messages
and Logging.Loggers
Messaging
feels kinda messy for my. But if u want it in the same folder that is okay for me.Did you make any code changes other than moving things around? If so let me know so I can review them too :)
I made some changes in codes that used the same functionality but has multiple functions for it, like the things about the loggers, but I already sended that to u on Discord. As for now I didn't really make "high" changes in the code because my main objective first was to separate and move some classes. When I have the new changes ready I am gonna look through the code again to see if there are some changed I can already apply like the ones I did with the logger.
When my new commits have been approved, I'll add more examples and add some more documentation especially the Migrate to v1.1
because it is needed ofcourse.
I am gonna look later into the things we discussed on Discord about message-listener and the possibly new features they have added in the Game SDK
Sorry for the delay. I wanted to do it sooner but got stuck in another project. I have applied all the proposed changes that u have requested. I also did everything u considered to do except the Messaging.Messages
and Logging.Loggers
since I haven't gotten an clear answer on that. If u still want all the messages
and loggers
in the same folder, I'll apply those changes.
I have added a new method in DiscordRpcClient.cs
that clones the current RichPresence and checks if the client has been initialized.
private RichPresence CloneCurrentPresence()
{
// Check if the client has been initialized
if (!IsInitialized) throw new UninitializedException();
// Close the presence
RichPresence presence;
lock (_sync)
{
presence = CurrentPresence == null ? new RichPresence() : CurrentPresence.Clone();
}
return presence;
}
The code was used in different methods in the class so I made an private function for it. If it ever needs to be changed, it only needs change in one place.
When this code-base has been approved the following things will be added:
This PR contains everything mentioned in https://github.com/Lachee/discord-rpc-csharp/issues/176
Please note that this is currently still a draft.
This PR will be in the following order: