Clan Requests by MrDetonia · Pull Request #856 · sebastian-heinz/Arrowgene.DragonsDogmaOnline

MrDetonia · 2025-05-26T20:46:09Z

Continues work on Clan Search by implementing the Clan Request feature. This allows characters to request access to a clan, with approval performed by clan owners or clan members with appropriate permissions set. The scouting system remains unimplemented.

Changes:

New table "ddon_clan_requests" added to the db schema. Version incremented and migration strategy written.
Handlers and related packet structures added for registering, cancelling, approving/rejecting, and listing clan join requests.
Clan permissions are respected (+ minor fix to clan search so that non-public clans are not listed).
New scheduled weekly task to remove stale requests older than 30 days (as suggested in-game(.
Changes to ClanManager's JoinClan so that offline characters can be added to a clan, and notified on next login.

Quirks / potential issues:

One character may only have up to one clan request at a time. This is slightly non-intuitive but the client provides feedback to that effect. The limitation here is in the client, not the server implementation (my original approach was to allow arbitrary numbers of requests per character).
Testing has been performed with sqlite and pgsql but only using various configurations of 2 test characters. A larger test environment (with more characters + clan members) may be beneficial to ensure there are no undesired behaviours.

Checklist:

The project compiles
The PR targets develop branch

…ts. Notify users on login when clan request approved.

RyanYappert · 2025-05-27T05:47:47Z

+            "clan_id", "requester_character_id", "created", "approved"
+        };
+
+        private readonly string SqlInsertClanRequest = $"""


Today I learned that C# will let you combine interpolated strings and raw strings.

RyanYappert · 2025-05-27T05:48:43Z

+    public class ClanRequestMigration : IMigrationStrategy
+    {
+        public uint From => 43;
+        public uint To => 44;


This may get bumped to 45 depending on the cadence of #822

RyanYappert · 2025-05-27T05:51:09Z

+        public bool CheckPermission(uint characterId, ClanPermission perm)
+        {
+            var (clanId, member) = ClanMembership(characterId);
+            var splitPerms = IntToEnums<ClanPermission>((int)member.Permission);


I know I wrote this stupid IntToEnums thing but apparently it's supposed to be a [Flags] Enum and then it just works without all this extra machinery.

RyanYappert · 2025-05-27T05:52:27Z

+
+            if (shouldDelete)
+            {
+                Server.Database.ExecuteInTransaction(conn =>


There's likely a way we can rearrange this logic such that a second transaction is not required.

Marking things as done in a commit-to-be. Has been reworked into a single transaction with network comms outside.

RyanYappert · 2025-05-27T05:55:58Z

+
+namespace Arrowgene.Ddon.Shared.Entity.PacketStructure
+{
+    public class C2SClanClanJoinRequest : IPacketStructure


C2SClanClanRegisterJoinReq

Marking things as done in a commit-to-be. Renamed.

RyanYappert · 2025-05-27T06:02:02Z

+            {
+                Server.Database.InsertClanRequest(request.ClanId, client.Character.CharacterId, conn);
+            });
+            return new S2CClanClanJoinRes();


This feels like it should be sending a notice to people who are currently online and can handle the application.

RyanYappert · 2025-05-27T06:02:29Z

+
+namespace Arrowgene.Ddon.GameServer.Handler
+{
+    public class ClanClanJoinRequestHandler : GameRequestPacketHandler<C2SClanClanJoinRequest, S2CClanClanJoinRes>


See comments about the misnamed packets, which would render this ClanClanRegisterJoinHandler

Marking things as done in a commit-to-be. Renamed.

RyanYappert · 2025-05-27T06:04:16Z

+
+namespace Arrowgene.Ddon.GameServer.Handler
+{
+    public class ClanClanApproveJoinRequestHandler : GameRequestPacketHandler<C2SClanClanApproveJoinReq, S2CClanClanApproveJoinRes>


ClanClanApproveJoinHandler

Marking things as done in a commit-to-be. Renamed.

RyanYappert · 2025-05-27T06:05:15Z

+
+namespace Arrowgene.Ddon.GameServer.Handler
+{
+    public class ClanClanCancelJoinRequestHandler : GameRequestPacketHandler<C2SClanClanCancelJoinReq, S2CClanClanCancelJoinRes>


ClanClanCancelJoinHandler

Also this has a notice associated with it, so you likely need to be notifying other clients that the request has been cancelled.

RyanYappert · 2025-05-27T06:10:50Z

+                    UInt32 clanId = Server.Database.GetClanRequestsByCharacter(request.CharacterId, conn).First().ClanId;
+                    Server.ClanManager.JoinClan(request.CharacterId, clanId, conn);
+
+                    if (Server.ClientLookup.GetClientByCharacterId(request.CharacterId) == null)


This is made somewhat more complicated by the fact that this player may be online, but on a different channel. You can semi-reliably find them with Server.RpcManager.FindPlayerById(request.CharacterId), but you'll have to send an RPC message to their server (if FindPlayer returns >0) to tell them that they've been accepted. You'll have to add a new command to PacketRoute to accommodate a packet for a single person.

alborrajo · 2025-05-27T15:08:18Z

    private const string DefaultSchemaFile = "Script/schema_sqlite.sql";

-    public const uint Version = 43;
+    public const uint Version = 44;


Same as #822, we have to be careful with the merged

alborrajo · 2025-05-27T15:11:38Z

    bool DeleteClanBaseCustomization(uint clanId, byte type, DbConnection? connectionIn = null);
    List<CDataClanSearchResult> SearchClans(CDataClanSearchParam param, DbConnection? connectionIn = null);
+    bool InsertClanRequest(uint clanId, uint characterId, DbConnection? connectionIn = null);
+    List<CDataClanJoinRequest> GetClanRequestsByCharacter(uint characterId, DbConnection? connectionIn = null);


Considering there can only be one application, shouldn't these return a single entity instead of a list?

alborrajo · 2025-05-27T15:13:50Z

    public abstract CDataClanMemberInfo GetClanMember(uint characterId, DbConnection? connectionIn = null);
    public abstract bool UpdateClanMember(CDataClanMemberInfo memberInfo, uint clanId, DbConnection? connectionIn = null);
+    public abstract bool InsertClanRequest(uint clanId, uint characterId, DbConnection? connectionIn = null);
+    public abstract List<CDataClanJoinRequest> GetClanRequestsByCharacter(uint characterId, DbConnection? connectionIn = null);


alborrajo · 2025-05-27T15:15:37Z

+                character = Server.CharacterManager.SelectCharacter(characterId, false, connectionIn);
+                if (character == null)
+                {
+                    Logger.Error("Tried to add offline character to clan but select character query failed.");


This should probably throw a ResponseErrorException with an appropriate error code so the user can see an error happening instead of having the client follow along thinking it succeeded

MrDetonia added 2 commits May 26, 2025 20:33

Update db schema to 44, clan requests.

cdbc122

Implement clan request system. Add task to clean up stale clan reques…

2458cf0

…ts. Notify users on login when clan request approved.

RyanYappert requested changes May 27, 2025

View reviewed changes

alborrajo reviewed May 27, 2025

View reviewed changes

alborrajo mentioned this pull request May 27, 2025

Feat: Quest Schedule ID Rework + Board Quest Rotation #822

Merged

2 tasks

Conversation

MrDetonia commented May 26, 2025

Changes:

Quirks / potential issues:

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants